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

copy the dtypes module to the namedarray package. #8250

Merged
merged 18 commits into from
Oct 4, 2023

Conversation

andersy005
Copy link
Member

@andersy005 andersy005 commented Sep 28, 2023

this PR copies the dtypes module from the xarray.core package to the xarray.namedarray package. It also recreates the ReprObject class from the utils module within the xarray.namedarray package

These changes are part of the ongoing work in:


  • towards NamedArray tracking issue #8238
  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
  • New functions/methods are listed in api.rst

xarray/core/dtypes.py Outdated Show resolved Hide resolved
@andersy005 andersy005 changed the title reorganizing the dtypes module to be located in the namedarray package. reorganize the dtypes module to be located in the namedarray package. Sep 28, 2023
@andersy005 andersy005 marked this pull request as ready for review September 28, 2023 18:01
@@ -66,3 +66,26 @@ def to_0d_object_array(value: typing.Any) -> np.ndarray:
result = np.empty((), dtype=object)
result[()] = value
return result


class ReprObject:
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is not considered np.typing.ArrayLike, therefore I don't like it. Is it possible to avoid it?

Example error from #8211:

 xarray/core/dataarray.py: note: In member "__init__" of class "DataArray":
xarray/core/dataarray.py:414: error: Incompatible default for argument "data" (default has type "ReprObject", argument has type "T_DuckArray | _SupportsArray[dtype[Any]] | _NestedSequence[_SupportsArray[dtype[Any]]] | bool | int | float | complex | str | bytes | _NestedSequence[bool | int | float | complex | str | bytes]")  [assignment]

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see why the constructor has a default value of "NA"

@andersy005 andersy005 changed the title reorganize the dtypes module to be located in the namedarray package. copy the dtypes module to the namedarray package. Sep 28, 2023
Copy link
Contributor

@Illviljan Illviljan left a comment

Choose a reason for hiding this comment

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

Add typing. mypy might fail after some suggestions

xarray/namedarray/dtypes.py Outdated Show resolved Hide resolved
xarray/namedarray/dtypes.py Outdated Show resolved Hide resolved
xarray/namedarray/dtypes.py Outdated Show resolved Hide resolved
xarray/namedarray/dtypes.py Outdated Show resolved Hide resolved
xarray/namedarray/dtypes.py Outdated Show resolved Hide resolved
xarray/namedarray/dtypes.py Outdated Show resolved Hide resolved
xarray/namedarray/dtypes.py Outdated Show resolved Hide resolved
xarray/namedarray/dtypes.py Outdated Show resolved Hide resolved
@andersy005 andersy005 added needs review topic-NamedArray Lightweight version of Variable labels Sep 29, 2023
@andersy005
Copy link
Member Author

@Illviljan / @dcherian, I'm leaning towards merging this PR unless there are any objections and/or edits

@Illviljan
Copy link
Contributor

I was hoping to get #8241 merged before to see the remaining type issues.

@dcherian
Copy link
Contributor

dcherian commented Oct 4, 2023

merged before to see the remaining type issues.

Let's merge with some type ignores. There are many more major issues to be handled, and time is running out...

@dcherian dcherian merged commit 8d54acf into pydata:main Oct 4, 2023
23 of 26 checks passed
dcherian added a commit to dcherian/xarray that referenced this pull request Oct 14, 2023
* upstream/main: (46 commits)
  xfail flaky test (pydata#8299)
  Most of mypy 1.6.0 passing (pydata#8296)
  Rename `reset_encoding` to `drop_encoding` (pydata#8287)
  Enable `.rolling_exp` to work on dask arrays (pydata#8284)
  Fix `GroupBy` import (pydata#8286)
  Ask bug reporters to confirm they're using a recent version of xarray (pydata#8283)
  Add pyright type checker (pydata#8279)
  Improved typing of align & broadcast (pydata#8234)
  Update ci-additional.yaml (pydata#8280)
  Fix time encoding regression (pydata#8272)
  Allow a function in `.sortby` method (pydata#8273)
  make more args kw only (except 'dim') (pydata#6403)
  Use duck array ops in more places (pydata#8267)
  Don't raise rename warning if it is a no operation (pydata#8266)
  Mandate kwargs on `to_zarr` (pydata#8257)
  copy  the `dtypes` module to the `namedarray` package. (pydata#8250)
  Add xarray-regrid to ecosystem.rst (pydata#8270)
  Use strict type hinting for namedarray (pydata#8241)
  Update type annotation for center argument of dataaray_plot methods (pydata#8261)
  [pre-commit.ci] pre-commit autoupdate (pydata#8262)
  ...
@andersy005 andersy005 deleted the move-dtypes-to-namedarray branch February 12, 2024 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review topic-NamedArray Lightweight version of Variable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants