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

rename get_rows to take #339

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

MarcoGorelli
Copy link
Contributor

In pandas, there's .iloc, but the consortium doesn't want that

Polars has .gather

pytorch also uses gather for this kind of operation:

>>> t = torch.tensor([1, 2, 3, 4])
>>> torch.gather(t, 0, torch.tensor([3, 2]))
tensor([4, 3])

tensorflow has gather, which does the same kind of thing https://www.tensorflow.org/api_docs/python/tf/gather

I've not seen get_rows anywhere, it's definitely not a common name. Personally I'd rather go with common existing names than create new ones

I promise I'll stop bikeshedding after February (when we aim to make the first non-beta release), trying to get it all out before then

Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm +1 to gather as well

@jorisvandenbossche
Copy link
Member

FWIW, not being familiar with a gather method before, I don't find that name necessarily obvious (I would rather guess it does something like "collect").
If you want a name that is already used by other libraries, take is another obvious candidate, which is being used by pandas, numpy and also the array api (I am not sure if I would find take necessarily a more obvious name if I would not be familiar with it already (hard to say because I am)).

@MarcoGorelli
Copy link
Contributor Author

thanks for your inputs - I'm OK with either, I think they're both improvements over get_rows

@MarcoGorelli
Copy link
Contributor Author

looks like R has gather, but it does something completely different http://statseducation.com/Introduction-to-R/modules/tidy%20data/gather/

maybe take is better then

@MarcoGorelli MarcoGorelli changed the title rename get_rows to gather rename get_rows to take Dec 8, 2023
@MarcoGorelli
Copy link
Contributor Author

@kkraus14 you OK with take too?

@MarcoGorelli
Copy link
Contributor Author

thanks all, let's get this in then

@MarcoGorelli MarcoGorelli merged commit cf6eab6 into data-apis:main Dec 8, 2023
4 checks passed
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.

3 participants