Skip to content

uxarray.concat implementation incomplete/buggy and needs a docstring #1642

Description

@Sevans711

Version

v2026.7.0

How did you install UXarray?

Source

What happened?

uxarray defines concat() in its public API, however the implementation is confusing/incomplete/buggy and does not have any docstring. Examples of issues (aside from missing docstring):

  1. The implementation seems to attempt to support objects other than UxDataArray and UxDataset; instead of explicit type-checking it only checks "do the objects all have a uxgrid attribute? If yes, proceed." Is this intentional? There is no documentation to clarify so I can't tell, but it's not necessarily what I would have guessed. Relevant to comment: Improve uxgrid=None docs and error-handling for UxDataArray & UxDataset. #1640 (comment)
  2. Using objects with equal uxgrids which are not exactly the same object (via is) incorrectly cause crash.
  3. UxDataArrays are not actually supported; the method always attempts to convert to UxDataset even if inputs are all UxDataArrays. This leads to a confusing error: TypeError: unhashable type: 'UxDataArray'.
  4. uxarray.concat() does not appear anywhere in the test suite. (It also isn't used anywhere internally throughout uxarray, which might be why these issues haven't been reported yet…?)

What did you expect to happen?

  1. uxarray.concat needs a docstring.
  2. Use explicit type-checking (e.g. isinstance(obj, (UxDataArray, UxDataset))). Or (probably less likely to be the desired solution), clarify in docstring that this method attempts to support other objects too, as long as they have a certain combination of characteristics (one of which is "has a uxgrid attribute")
  3. uxgrids should be compared via == instead of is. This would match the existing pattern from UxDataArray's calculus methods.
  • (2a) If efficiency is a concern, consider adding a check like if self is other: return True inside Grid.__eq__.
  • (2b) Similarly to those comparisons, the error should be changed to a GridsMismatchError if the grids are not equal. Missed this during Add custom error types in uxarray #1621, possibly because it looked like an object id comparison rather than a grid equality test.
  1. UxDataArrays should be supported in a way that naturally extends how xarray.concat handles DataArrays. E.g., if the inputs are all UxDataArrays, the output should be as well.
  2. As apart of public API, there should definitely be at least a few tests to ensure uxarray.concat works as expected.

Can you provide a MCVE to repoduce the bug?

import uxarray as ux

# -- issue 1 --
class Foo():
    def __init__(self, uxgrid):
        self.uxgrid = uxgrid
foo1 = Foo(7)
foo2 = Foo(7)  # also 7 because: want to use the same uxgrid value in both cases.
ux.concat([foo1, foo2])
# >> TypeError: concat() missing 1 required positional argument: 'dim'
# (expected: "TypeError: expected UxDataArray or UxDataset; got object of type Foo")


# -- issue 2 --
arrA = ux.tutorial.open_dataset("outCSne30-vortex")['psi']
arrB = arrA.copy()
ux.concat([arrA, arrB], dim='new_dim')
# >> ValueError: Object at index 1 has a different 'uxgrid' attribute.
# (expected: should be able to concat these; the grids are definitely equivalent...)


# -- issue 3 --
arrA = ux.tutorial.open_dataset("outCSne30-vortex")['psi']
arrB = arrA + 10
ux.concat([arrA, arrB], dim='new_dim')
# >> TypeError: unhashable type: 'UxDataArray'
# (expected: should be able to concat these... result should be a UxDataArray with
#  arrA values at result.isel(new_dim=0) and arrB values at result.isel(new_dim=1).)

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdocumentationImprovements or additions to documentation

    Type

    Projects

    Status
    📚 Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions