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 array correspondence simpler, slightly safer, and make API more numpy-like #49

Merged
merged 10 commits into from
Feb 3, 2023

Conversation

tovacinni
Copy link
Contributor

This new array correspondence makes:

  1. The API more numpy-like. The axis now specifies the dimension to do the correspondence search over.
  2. Adds some asserts for safety.
  3. Some simple comments to explain what the code is doing.
  4. Replace the byte comparison (which will amount to pointer comparisons to objects) with multi-dim np.unique which is a bit more readable and works for single-dim object arrays.

Unfortunately np.unique doesn't work well for multi-dim object arrays... but these would've failed in the any case in the older version so at least it's not a regression.

@odedstein odedstein self-assigned this Jan 17, 2023
@odedstein
Copy link
Collaborator

odedstein commented Jan 17, 2023

@tovacinni I changed a few things. Please tell me whether you are ok with them (I'm still working on the failing unit tests).
Since we have a breaking change with this PR (the axis convention), we'll try to wait for the next 0.x.0 version to put this in main.

@tovacinni
Copy link
Contributor Author

Sounds good with me!

@sgsellan sgsellan added breaking change This PR is finished but includes a breaking change so it will be merged for the next 0.x.0 release enhancement New feature or request labels Jan 17, 2023
@odedstein odedstein merged commit 51e274d into sgsellan:main Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This PR is finished but includes a breaking change so it will be merged for the next 0.x.0 release enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants