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

bugfix/use-data.data-in-open_scene #69

Conversation

psobolewskiPhD
Copy link
Collaborator

Description

If a single scene image is opened, the return is data.data which is just the array from the xarray.DataArray
If a multi scene file is opened (using the scene selector widget) the return from the open_scene is an image layer with data, the actual xarray.DataArray.

Napari actually handles this just fine, but some downstream plugins do not, expecting numpy arrays (e.g. napari-matplotlib). This is sort of something they should be better at handling, perhaps using np.asarray, but at the same time I feel like these two should work the same?
I think this plugin should/could be the default beyond builtins, so probably standardizing on the array and not xarray.DataArray is best, so probably data.data is preferred as a default behavior.
As the person partially responsible for the scene selector, I think this was an oversight on my part...

So in this PR I just make that small change and update the test accordingly.
At the same time, I can understand just returning the xarray.DataArray. But then for consistency, the single scene should just return data.

Pull request recommendations:

  • Name your pull request your-development-type/short-description. Ex: feature/read-tiff-files
  • Link to any relevant issue in the PR description. Ex: Resolves [admin/build-py39 #12], adds tiff file format support
  • Provide context of changes.
  • Provide relevant tests for your feature or bug fix: I updated the test for the change.
  • Provide or update documentation for any feature added by your pull request.

Thanks for contributing!

@psobolewskiPhD
Copy link
Collaborator Author

Lint CI fail is unrelated, hopefully fixed by #70

@evamaxfield
Copy link
Collaborator

Seems fine to me! Thanks! Also @psobolewskiPhD do you mind if I give you write access to this repo?

I unfortunately haven't had time to work on imaging / cell science stuff in a long time. Happy to still review but I don't see myself as a large code author for the foreseeable future...

@evamaxfield
Copy link
Collaborator

Reason being: if you have write access you can publish releases after something merges.

@evamaxfield evamaxfield merged commit 52c6648 into AllenCellModeling:main Mar 8, 2023
@psobolewskiPhD
Copy link
Collaborator Author

Um, I don't mind I guess, if you think it's helpful!
I'll definitely need to bone up on releases and all that—I've never done that.

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