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

Minor fixes to checkbounds_linear_indices and getindex for ranges #20686

Merged
merged 2 commits into from
Feb 20, 2017

Conversation

timholy
Copy link
Member

@timholy timholy commented Feb 20, 2017

No description provided.

This happened to work because start returns the same thing as first for our two OrdinalRange types in base, but it's not guaranteed for all possible OrdinalRange types.
@kshyatt kshyatt added the collections Data structures holding multiple items, e.g. sets label Feb 20, 2017
@timholy timholy merged commit 30a215d into master Feb 20, 2017
@timholy timholy deleted the teh/linear_indices_amb branch February 20, 2017 23:35
@tkelman
Copy link
Contributor

tkelman commented Feb 21, 2017

happened to work because start returns the same thing as first for our two OrdinalRange types in base, but it's not guaranteed for all possible OrdinalRange types

should one be added for testing, like with the TestHelpers OffsetArray?

@timholy
Copy link
Member Author

timholy commented Feb 21, 2017

It's much more of a theoretical problem than OffsetArray, which tests things that are actually running out there in the wild. Not that it would be a bad thing, of course, but doesn't seem like a high priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collections Data structures holding multiple items, e.g. sets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants