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 explicit that axes comparisons in tests ignore indices #357

Merged
merged 2 commits into from
Aug 18, 2024

Conversation

matthias314
Copy link
Contributor

This PR was suggested by @nsajko in JuliaLang/julia#54825. Currently Julia compares ranges only by comparing their values, not their indices, although they should also be taken into account for AbstractVector. The tests in OffsetArrays.jl rely on this special behavior. This PR makes this explicit. It does not fix the broken mapreduce test mentioned in #353.

@jishnub
Copy link
Member

jishnub commented Aug 17, 2024

We may use no_offset_view here to generate the offset-free axis.

@matthias314
Copy link
Contributor Author

Good point. I'm now using no_offset_view, and I've renamed axes_values to no_offset_axes.

If one wanted to have it really fancy, one could define a new operator, for example , that compares ranges and tuples of ranges without looking at the indices. That's not done at present.

Copy link

codecov bot commented Aug 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.67%. Comparing base (3910605) to head (e821865).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #357   +/-   ##
=======================================
  Coverage   98.67%   98.67%           
=======================================
  Files           6        6           
  Lines         453      453           
=======================================
  Hits          447      447           
  Misses          6        6           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matthias314
Copy link
Contributor Author

The failing test is the unrelated mapreduce issue JuliaLang/julia#55506.

@jishnub
Copy link
Member

jishnub commented Aug 18, 2024

It's a different issue related to floating-point precision, but indeed unrelated to this PR.

@jishnub jishnub merged commit 341e990 into JuliaArrays:master Aug 18, 2024
16 of 17 checks passed
@matthias314 matthias314 deleted the m3/ranges-equal-tests branch August 18, 2024 19:09
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

Successfully merging this pull request may close these issues.

2 participants