pyimagej
pyimagej copied to clipboard
xarray.DataArray `imagej` metadata and improved axis/scale logic
This PR improves how we assign the imagej CalibratedAxis
subclass of a given axis when converting an xarray.DataArray
to a net.imagej.Dataset
. Currently only DefaultLinearAxis
and EnumeratedAxis
are supported, with EnumeratedAxis
being preferred if available. This behavior can be problematic as some projects expect to see a DefaultLinearAxis
instead of an EnumeratedAxis
. This PR changes this behavior, by assigning DefaultLinearAxis
as the default axis unless the associated imagej
metadata indicates otherwise. For more specific details check out this change log:
- Added an
imagej
metadata attribute toxarray.DataArray
s that are made via PyImageJ (i.e. results fromij.py.to_xarray()
andij.py.from_java()
). Theimagej
metadata attribute is a dictionary that contains per axis information such asscale
,origin
and theCalibratedAxis
subclass type. - Added matching support for the rest of the
CalibratedAxis
subclasses such asPolynomialAxis
. Even though these are not used (as far as we know) by any plugins/projects the I thought it was best to be thorough and include all the subclasses. The refactoring I did forimagej.dims._assign_axes()
made this addition easy. - Add a check for singleton dimensions when calculating scale for the fallback linear axis.
- Refactor
imagej.dims._assign_axes()
(now uses theimagej
metadata attribute if present to get axes information). - Removed deprecated private functions.
- Misc code cleanup.
Codecov Report
Patch coverage: 76.27
% and project coverage change: -0.14
:warning:
Comparison is base (
f02b272
) 76.95% compared to head (615ee25
) 76.82%.
:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
Additional details and impacted files
@@ Coverage Diff @@
## main #247 +/- ##
==========================================
- Coverage 76.95% 76.82% -0.14%
==========================================
Files 16 18 +2
Lines 1870 1963 +93
==========================================
+ Hits 1439 1508 +69
- Misses 431 455 +24
Impacted Files | Coverage Δ | |
---|---|---|
src/imagej/metadata/axis.py | 61.53% <61.53%> (ø) |
|
src/imagej/dims.py | 61.34% <67.74%> (-1.77%) |
:arrow_down: |
src/imagej/_java.py | 81.60% <71.11%> (-5.91%) |
:arrow_down: |
src/imagej/convert.py | 84.97% <100.00%> (+0.14%) |
:arrow_up: |
src/imagej/metadata/__init__.py | 100.00% <100.00%> (ø) |
|
tests/test_image_conversion.py | 98.84% <100.00%> (+<0.01%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
@elevans Is this ready for review now? Or are you still working on it?
I'm still working on this actually. There are a few changes I want to add that I was planning to complete on my flight back home from Japan!
I'm converting this PR back to a draft as it needs some re-working again before its ready for review again.
I have unfortunately been busy putting out other fires and I haven't had a chance to really get this PR off the ground to where it should be in my opinion. But my time away has allowed me to think more about how to tackle this problem. So far the work I've done creates a fancy dictionary that gets attached to the xarray.DataArray
's global attributes. This is fine and dandy but as soon as someone slices the array into a new shape the metadata is out of sync. This seems like a non-viable approach going forward for sophisticated image metadata for our users.
Having said all that I think moving forward the best approach is to subclass the xarray.DataArray
after converting from Java and add our own methods to the array. For example I just committed https://github.com/imagej/pyimagej/pull/247/commits/ccee1b3dbe11167eb213a97c5932a3203ace77a8 which does the same isRGB
check that net.imagej.DefaultDataset
does. I originally added this to update an RGB
key in the out going xarray.DataArray
metadata dict. A better approach in my opinion would be to convert this logic to work on the xarray.DataArray
itself as a method of our subclassed xarray.DataArray
.
EDIT: After doing some reading the xarray team discourages subclassing xarray.DataArray
because (1) its not supported and (2) there are places where internally the only the base xarray.DataArray
object is returned, thus breaking your extended functionality. Thankfully the xarray team is great and has a register_dataarray_accessor()
method that we can use to add our custom methods. See this documentation for more details.