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

fix: numpy compatibility #18

Closed
wants to merge 2 commits into from

Conversation

ruseinov
Copy link

Unfortunately, numpy does not currently work with ndarray 0.16 as seen here.

Since 0.9.0 version is using an older version of image crate - I have decided to fork this for the time being.

Please let me know if we could perhaps release this as 0.10.0-numpy-compat or similar to avoid having to supply a git dependency. Then I'll make appropriate changes to Cargo.toml.

@vadixidav
Copy link
Member

@ruseinov Thank you for the contribution. I think for the time being I want to avoid having to maintain individual versions to support different library sets. However, I welcome anyone coming across this to use your fork.

If you need to patch it so that other crates are forced to use it as well, you can do the following:

[patch.crates-io]
nshare = { git = "https://github.com/ruseinov/nshare.git", rev = "365650125139ee92d73b5da909acf9ccd702b3ee" }

Adding this to your Cargo.toml will override this dependency, even for other crates depending on nshare. If those crates actually use ndarray 0.16` though, it most likely wont work, and you will encounter build errors.

@vadixidav vadixidav closed this Sep 15, 2024
@ruseinov
Copy link
Author

@vadixidav I'm curious why let shape = (channels as usize, height as usize, width as usize);

For example, Python's Pillow puts channel last, which actually makes total sense. If we look at how OpenCV works - it's exactly the same.

Would you consider a contribution that changes that?

@ruseinov
Copy link
Author

ruseinov commented Sep 17, 2024

I have introduced the change in that same PR, but I can isolate it if the contribution is of interest.
Here's the commit: ruseinov@7d01237

@vadixidav
Copy link
Member

@vadixidav I'm curious why let shape = (channels as usize, height as usize, width as usize);

The reason for this is that the image crate stores images with red, green, and blue values interleaved rather than having separate 2d arrays for each color. The way the code is written, it will correctly load data from the image crate using the standard convention of ndarray where the major axis is the final item and minor axis is the first item. If you need the axes to be reshuffled, ndarray has methods on ArrayBase you can use to change which axis is which arbitrarily. The implementation here is just ensuring that both crates have the same data before and after the conversion.

@vadixidav
Copy link
Member

A resource that might help is the following https://docs.rs/ndarray/latest/ndarray/type.Array.html#method.push_column. Note that when you append an axis to the end, you are appending the one with the longest stride. Therefore, the leftmost axis is the one with the shortest stride, hence color in this case. This is probably confusing, but the axes are listed in minor to major order. This may be what tripped you up.

@ruseinov
Copy link
Author

A resource that might help is the following https://docs.rs/ndarray/latest/ndarray/type.Array.html#method.push_column. Note that when you append an axis to the end, you are appending the one with the longest stride. Therefore, the leftmost axis is the one with the shortest stride, hence color in this case. This is probably confusing, but the axes are listed in minor to major order. This may be what tripped you up.

Thanks for the explanation!
In this case I'm trying to keep the results consistent with their Python alternatives, I'll double-check that the behaviour is correct.

@vadixidav
Copy link
Member

The behavior is likely correct, but it wont follow the standard convention from ndarray. It sounds like you want things that help adapt to the Python ecosystem, presumably with PyO3? This crate helps n-dimensional array crates talk to each other in Rust, but it may be that you need a crate that would help Rust n-dimensional arrays better adapt to the Python ecosystem. I know that a crate that just had a lot of typical operations to help bridge the two worlds could be helpful to people, so perhaps another crate is needed there. I intend to keep nshare the way it is, where it only makes the minimal changes to directly interpret data between n-dimensional crates, keeping everything the same. If additional transformations need to be made afterwards to adapt axes and other things to Python, I think that would be an excellent job for an additional crate.

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