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

icount masks the icount function from the iterators package, but does not include iterators::icount's functionality #39

Closed
gwerbin opened this issue Jul 30, 2015 · 5 comments

Comments

@gwerbin
Copy link

gwerbin commented Jul 30, 2015

The iterators version of icount generates integer sequences with a fixed step size of 1, starting from 1, either forever (with no argument) or until a specified stopping point is reached. The itertools2 version generates infinite integer sequences with a specified step size.

There are two problems with this:

  1. icount is part of the iterators package and not itertools, so I don't feel it's appropriate to mask.
  2. Loading itertools2 will drastically alter the behavior of the function, necessitating defensive ::s everywhere.

I understand that your icount function is closer to Python's than itertools::icount. Your thoughts?

And by the way, a brief comparison of itertools and itertools2 would be a nice addition to the readme.

@ramhiser
Copy link
Owner

You make some good points. I'll address them later today.

@ramhiser
Copy link
Owner

Before I address the icount concern, you make a good point about differentiating itertools vs itertools2 in the readme. I'll post an issue.

@ramhiser
Copy link
Owner

First off, this is an issue that crops up with various packages, e.g., dplyr vs plyr, reshape vs reshape2. A quick Google search for r package mask function yields multiple results suggesting to use the :: option. This Quora post discusses the issue in detail.

@gwerbin Suggestions for improvement?

@gwerbin
Copy link
Author

gwerbin commented Jul 31, 2015

@ramhiser I'm aware of how masking works in R, and it's impossible to always account for it. However in both of the cases you mentioned, the newer package offers the functionality that was masked, but in another form. The main issue here isn't so much the masking as it is the inconsistency, and the fact that itertools2 has no equivalent to iterators::icount. The easiest solution in my opinion would be to add an optional stopping point to itertools2::icount.

@ramhiser
Copy link
Owner

My apologies if I sounded pedantic. I get what you are saying, but I point you to the classic reshape::melt vs reshape2::melt example as a case where functionality differs slightly between functions that can get masked.

Regardless, I like your idea of the optional stopping point. I'll add icount(..., stop=NULL). Example usage: as.list(icount(stop=5)) equivalent to as.list(1:5).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants