Skip to content

MAINT: remove unused nd_grid __len__. #12159

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 17, 2018
Merged

MAINT: remove unused nd_grid __len__. #12159

merged 2 commits into from
Oct 17, 2018

Conversation

tylerjereddy
Copy link
Contributor

@tylerjereddy tylerjereddy commented Oct 12, 2018 •
edited
Loading

Add a test to trigger uncovered path in nd_grid corresponding to len probe of empty mgrid instance.

Now, just removing the unused method

@tylerjereddy tylerjereddy mentioned this pull request Oct 12, 2018
Closed
@mattip
Copy link
Member

mattip commented Oct 14, 2018

close/open to trigger windows builds

@mattip mattip closed this Oct 14, 2018
@mattip mattip reopened this Oct 14, 2018
@eric-wieser
Copy link
Member

eric-wieser commented Oct 15, 2018 •
edited
Loading

This method doesn't make any sense - we should just remove nd_grid.__len__. It seems it's been there from the beginning of time (f897e1f), but I can't think of any reason you would want it.

@tylerjereddy tylerjereddy changed the title TST: test for empty mgrid len MAINT: remove unused nd_grid __len__ Oct 15, 2018
@tylerjereddy
Copy link
Contributor Author

Revised based on feedback to remove __len__ from nd_grid

eric-wieser
eric-wieser approved these changes Oct 16, 2018
View reviewed changes
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

Maybe worthy of a compatibility note, that the expression len(np.mgrid) is now considered nonsensical and throws a TypeError.

@tylerjereddy
Copy link
Contributor Author

Added a compatibility note as requested -- ogrid is equally affected too.

@eric-wieser
Copy link
Member

eric-wieser commented Oct 17, 2018 •
edited
Loading

@rgommers: Does this need a deprecation cycle, or is it such a weird thing to do that we'd expect it to break no one?

@charris
Copy link
Member

charris commented Oct 17, 2018

I suppose len was there to compliment the use of indexing, which seems like an abuse of notation. If only there was a easier way to generate the slices, and the use of complex was always weird.

@charris charris merged commit 4126f66 into numpy:master Oct 17, 2018
@charris
Copy link
Member

charris commented Oct 17, 2018

Thanks Tyler.

@tylerjereddy tylerjereddy deleted the nd_grid_len_test branch October 17, 2018 23:16
@charris charris changed the title MAINT: remove unused nd_grid __len__ MAINT: remove unused nd_grid __len__. Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-wieser eric-wieser eric-wieser approved these changes

Assignees
No one assigned
Labels
03 - Maintenance component: numpy.lib
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tylerjereddy @mattip @eric-wieser @charris