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

Add extra overload for to_netcdf #8268

Merged
merged 4 commits into from
Dec 3, 2023
Merged

Conversation

jenshnielsen
Copy link
Contributor

The current signature does not match with pyright if a non-literal bool is passed.

This fixes type checking of code like this:

import xarray as xr

def myfunc(compute: bool) -> None:
    ds = xr.Dataset()
    ds.to_netcdf(path="myfile.nc",
                compute=compute)

with pyright.

See microsoft/pyright#6069 for context.

The current signature does not match with pyright
if a non literal bool is passed.
Copy link
Collaborator

@headtr1ck headtr1ck left a comment

Choose a reason for hiding this comment

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

There are still some Mypy errors. Maybe my comment fixed them, maybe not, maybe it creates new problems.

Also, feel free to add an entry in whats-new!

xarray/backends/api.py Outdated Show resolved Hide resolved
@headtr1ck headtr1ck added plan to merge Final call for comments topic-typing labels Dec 2, 2023
Copy link
Collaborator

@headtr1ck headtr1ck left a comment

Choose a reason for hiding this comment

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

I took the liberty to fix the open problems.
Good to go now!

On a side note, maybe I will open an issue over at mypy to request the feature that Union[Literal[True], Literal[False]] == bool

@headtr1ck
Copy link
Collaborator

On a side note, maybe I will open an issue over at mypy to request the feature that Union[Literal[True], Literal[False]] == bool

Already exists: python/mypy#14764

@headtr1ck headtr1ck merged commit b8b7857 into pydata:main Dec 3, 2023
31 of 33 checks passed
@headtr1ck
Copy link
Collaborator

Thanks!

@jenshnielsen
Copy link
Contributor Author

@headtr1ck thanks for finishing this. Sorry for dropping the ball on it

@jenshnielsen jenshnielsen deleted the to_netcdf_types branch December 4, 2023 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants