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

make require_one_based_indexing part of the API #31582

Closed
tpapp opened this issue Apr 2, 2019 · 4 comments
Closed

make require_one_based_indexing part of the API #31582

tpapp opened this issue Apr 2, 2019 · 4 comments

Comments

@tpapp
Copy link
Contributor

tpapp commented Apr 2, 2019

#30630 introduced the convenience function Base.require_one_based_indexing, which is very useful for conveying intent in assertions.

I propose that this is made part of the official API for, ideally, the 1.2 release. Not necessarily exported (to keep the namespace clean), just documented.

@tpapp
Copy link
Contributor Author

tpapp commented Apr 3, 2019

@KristofferC, can you explain why you don't think it is a good idea?

@KristofferC
Copy link
Member

has_offset_axes is already documented and anyone who needs this can just write their own function on top of that. The function has an ugly name, it will likely rarely be used and it doesn't seem worth exposing.

@mkitti
Copy link
Contributor

mkitti commented Jul 10, 2022

This suggests that we should remove mention of Base.require_one_based_indexing from the documentation and prefer has_offset_axes.

@johnnychen94
Copy link
Member

#43263 already documented it during the discussion on Yuri's blog so I figure this should be closed.

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

4 participants