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

to_xarray does not rename dimensions as expected #320

Closed
elevans opened this issue Dec 19, 2024 · 1 comment
Closed

to_xarray does not rename dimensions as expected #320

elevans opened this issue Dec 19, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@elevans
Copy link
Member

elevans commented Dec 19, 2024

I was working on #304 when I discovered that our ij.py.to_xarray() does not rename dimensions properly. When to_xarray() is given an xarray.DataArray with dimensions and a new dimension order (i.e. a list of new dimension names) the expected output is a rename of the dimensions. For example:

>>> xarr.shape
(10, 6, 50, 100, 3)
>>> xarr.dims
('t', 'pln', 'row', 'col', 'ch')
>>> xarr = ij.py.to_xarray(xarr, dim_order=['pln', 't', 'ch', 'col', 'row'])
>>> xarr.shape
(10, 6, 50, 100, 3)
>>> xarr.dims
('pln', 't', 'ch', 'col', 'row')

What to_xarray() with a new dimension order should do here is rename the dimensions while preserving the shape and the coordinates. It should not reshape the array. If a you want to do that, then the user should use xarray.DataArray.transpose().

When I try to do this dimension renaming with pyimagej built from main I get this ValueError:

>>> xarr = ij.py.to_xarray(xarr, dim_order=['pln', 't', 'ch', 'col', 'row'])
Traceback (most recent call last):
  File "/home/edward/Documents/workspaces/dev/issues/pyimagej/304/new_dims.py", line 21, in <module>
    xarr = ij.py.to_xarray(xarr, dim_order=new_dims)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/edward/Documents/repos/loci/pyimagej/src/imagej/__init__.py", line 525, in to_xarray
    return convert._rename_xarray_dims(data, dim_order)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/edward/Documents/repos/loci/pyimagej/src/imagej/convert.py", line 657, in _rename_xarray_dims
    xarr = xarr.swap_dims(dim_map)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/edward/miniforge3/envs/pyijdev/lib/python3.12/site-packages/xarray/core/dataarray.py", line 2581, in swap_dims
    ds = self._to_temp_dataset().swap_dims(dims_dict)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/edward/miniforge3/envs/pyijdev/lib/python3.12/site-packages/xarray/core/dataset.py", line 4538, in swap_dims
    raise ValueError(
ValueError: replacement dimension 'row' is not a 1D variable along the old dimension 'ch'

Here's a minimal script that reproduces this:

import imagej
import numpy as np
import xarray as xr

# initialize imagej
ij = imagej.init()

# create a dummy xarray
dims = ("t", "pln", "row", "col", "ch")
shape = (10, 6, 50, 100, 3)
coords = {
        "row": range(50),
        "col": range(100)
        }
narr = np.zeros(shape, dtype=np.uint16)
xarr = xr.DataArray(narr, dims=dims, coords=coords)                                            
                                                                                               
# rename the dimensions                                                                        
new_dims = ("pln", "t", "ch", "col", "row")                                                    
xarr = ij.py.to_xarray(xarr, dim_order=new_dims) 
@elevans elevans self-assigned this Dec 19, 2024
@elevans elevans added the bug Something isn't working label Dec 19, 2024
@elevans
Copy link
Member Author

elevans commented Dec 19, 2024

It looks like xarray is no longer deprecating xarray.DataArray.rename as this works and produces the desired result of just changing the dimension labels without changing the shape of the array.

dim_map = {
        "t": "pln",
        "pln": "t",
        "row": "ch",
        "col": "row",
        "ch": "col"
        }
xarr = xarr.rename(dim_map)

But at some point this method was flagged to be deprecated (see commit: c003095). I clearly missed this discussion indicating that the deprecation warning was actually planned on being dropped. I'm currently using xarray 2024.9.0 which does not produce this warning.

The remedy here is to revert back to our old logic which works as intended. Additionally I will add some extra tests that more rigorously shuffles the dimension names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant