-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix missing articles and grammer in docs #398
Conversation
@@ -16,7 +16,7 @@ look pretty similar in the data wrangling DSL. | |||
|
|||
## List of Access APIs | |||
|
|||
Here's a list of all APIs in order to increase safety. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose "in order to increase" is better here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it really isn't. We say the list is ordered by increasing safety. So the lower on the list, the safer it gets. We describe the order, not the reason ("in order to...").
|
||
Use this operation to change a formal type of [`DataFrame`](DataFrame.md) | ||
Use this operation to change the formal type of a [`DataFrame`](DataFrame.md) instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, better to go through all the docs and add instance to all places where it should be applied
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that would be best. There's however a lot of cases where this happens :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do that afterward though. Currently, I'm just going through your changes and checking and replacing grammar there. When I'm done, I'll do a ctrl+r on the entire thing :) Either here or a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, there are a lot of cases where I would change [`DataFrame`](DataFrame.md)
to data frame
, since it talks about the concept. In other cases, such as here, we explicitly talk about the type, so then I'd prefer instance
. Needs to have a thorough check, throughout the documentation; should probably be a separate PR
@Jolanrensen feel free to fix "instances" issue and merge it ASAP in the master branch without submitting on review, because I need to continue my work on the docs today and tomorrow |
I'll revert the grammar fixes in the code parts (well, running korro will do that). We should probably fix those in a separate PR if we want to. That said, missing articles in quick code comments are less of an issue, since compactness is often key there |
Same as #394 but moved inside the repo, so we can work together in the same branch.
Fixed #395
Edit: Okay, this is too large to fix in 1 simple PR, especially before the 0.11.0 release.
I'll merge the checked changes now and postpone the checks for later. The entire documentation needs a good check, in grammar, clarity and spelling, but this will take longer.