Skip to content
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

Closes #1521, fixes typo in between.Rd #1522

Merged
merged 1 commit into from
Feb 8, 2016
Merged

Closes #1521, fixes typo in between.Rd #1522

merged 1 commit into from
Feb 8, 2016

Conversation

MichaelChirico
Copy link
Member

Basically identical to #1515; with perfect foresight these would have been the same PR.

@arunsrinivasan
Copy link
Member

Could you please make changes specific to the issue you highlight? There's no ambiguity in the documentation as far as I can tell. Also what's orderable? The spacing in the code section is also on purpose. Matt doesn't like spaces between argument and value.

@MichaelChirico
Copy link
Member Author

@arunsrinivasan orderable means <= etc. are valid.

See 3 + 2i < 1 + 3i -- complex types are not orderable. More commonly, factor(1) < factor(2) is also outside the scope of between. I'll revert the spacing.

I switched to x %between% y to match the declaration of arguments in between.R. I understood that R CMD throws errors/warnings if there are undocumented arguments, which y currently is.

I've also added a quick sentence in Notes to point out the default value of incbounds in the infix usage.

@arunsrinivasan
Copy link
Member

I find that definition of orderable confusing. Where do you draw this definition from? The word doesn't exist in the dictionary. My first thought was if the vector can be ordered, i.e., if order(x) is valid..

Everything else makes sense. I'd suggest then to change to title of PR to "Improvements to between.Rd". This isn't just a typo fix, but quite a lot of nice improvements. Also you can split the news item (if you wish).

@MichaelChirico
Copy link
Member Author

@arunsrinivasan then you're using a poor dictionary ;-) OED definition 3 strikes quite close to what I have in mind ("That may be arranged in a series." in case it's paywalled).

But your confusion is still valid -- I'll add a few words to make it more clear what we have in mind as valid x.

Revision coming up tonight/tomorrow...

@arunsrinivasan
Copy link
Member

Hm, I see it here now. Note to self: don't rely on my mac's inbuilt dictionary.

Sure, no hurry. Take your time. Thanks!

reverting some spacing touch-ups

clariying

refinements
@MichaelChirico
Copy link
Member Author

@arunsrinivasan should be all set

@arunsrinivasan
Copy link
Member

Looks great!

arunsrinivasan added a commit that referenced this pull request Feb 8, 2016
Closes #1521, fixes typo in between.Rd
@arunsrinivasan arunsrinivasan merged commit af83025 into Rdatatable:master Feb 8, 2016
@MichaelChirico MichaelChirico deleted the bw_man branch February 8, 2016 23:08
@arunsrinivasan arunsrinivasan added this to the v1.9.8 milestone Feb 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants