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

Change GeometryDtype.na_value to None #2997

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jorisvandenbossche
Copy link
Member

@jorisvandenbossche jorisvandenbossche commented Sep 1, 2023

The GeometryDtype.na_value in practice is None (this is being returned when accessing a missing value), but the attribute itself it set to NaN.

In the past I remember that it gave some test failures, but let's see what it does now.

@jorisvandenbossche
Copy link
Member Author

And indeed, it gives a whole bunch of errors, it seems up to pandas 2.0. So we might be able to change this in the future when supporting pandas >= 2.0

@m-richards
Copy link
Member

And indeed, it gives a whole bunch of errors, it seems up to pandas 2.0. So we might be able to change this in the future when supporting pandas >= 2.0

I imagine defining na_value conditionally based on pandas version would be bad? (I could imagine this might be a problem for pickling but not sure)

@jorisvandenbossche
Copy link
Member Author

It might not be too bad to make na_value dependent on the pandas version, but I would personally like to avoid that I think

@m-richards
Copy link
Member

With pandas-dev/pandas#54930 this is no longer needed, and I suppose we can close this. (Though longer term it's perhaps still worth looking at setting the NA value to None for consistency once we only support pandas >=2)

@jorisvandenbossche jorisvandenbossche marked this pull request as draft May 18, 2024 08:36
@m-richards
Copy link
Member

Merged this with main again now that we support pandas >=2 only. Seems we may as well just do this for consistency, since the behaviour is that the na_value is None in practice (and maybe that let's the pandas side simplify tests if they want to)

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