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 broken doctest and tests on accessors #46

Merged
merged 4 commits into from
Jun 28, 2024

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Jun 23, 2024

Couple of tests to fix when running pytest --verbose --doctest-modules cupy_xarray:

  • On cupy_xarray.accessors.CupyDataArrayAccessor.as_cupy, output type should now be cupy.ndarray instead of cupy.core.core.ndarray
  • There was an error like ModuleNotFoundError: No module named 'xarray.core.pycompat' when running on xarray=2024.6.0. Need to apply a patch similar in concept to Fix broken dask_array_type import #24, but without importing xarray.core.pycompat (since it is a private function?). Specifically, change the from xarray.core.pycompat import dask_array_type line to:
    try:
        import dask.array
    
        dask_array_type = dask.array.Array
    except ImportError:
        dask_array_type = None

Partially cherry-picked from #45. These changes will need to be tested locally with a CUDA GPU device for now, until we can get CI working somehow.

Output type is now just `cupy.ndarray` instead of `cupy.core.core.ndarray`. Also updated pytest command in contributing.rst to include `--doctest-modules` flag.
@weiji14 weiji14 self-assigned this Jun 23, 2024
@weiji14 weiji14 marked this pull request as ready for review June 23, 2024 22:04
Comment on lines 8 to 9
dsk = DuckArrayModule("dask")
dask_array_type = dsk.type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just not do that

Suggested change
dsk = DuckArrayModule("dask")
dask_array_type = dsk.type
try:
import dask.array
dask_array_type = dask.array.Array
else:
dask_array_type = None

There is some new importlib way of doing this that I can't remember now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, done at 6229f99.

There is some new importlib way of doing this that I can't remember now

Do you mean importlib.util.find_spec? That would allow us to get to the dask.array module, but not the dask.array.Array type. Unless there's another function I'm missing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that one but maybe it's not a big deal here/

@weiji14 weiji14 merged commit 791c62d into xarray-contrib:main Jun 28, 2024
4 checks passed
@weiji14 weiji14 deleted the fix/test_accessors branch June 28, 2024 22:17
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