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

depr(python): Deprecate DataType.is_nested #11844

Merged
merged 2 commits into from
Oct 20, 2023
Merged

depr(python): Deprecate DataType.is_nested #11844

merged 2 commits into from
Oct 20, 2023

Conversation

stinodego
Copy link
Member

@stinodego stinodego commented Oct 19, 2023

I really like the idea of extending our DataType class with methods like is_nested() and is_integer() (and other groups). But these should not be properties - they should be (class)methods just like we have Series.is_integer() and DataFrame.is_empty() etc.

Unfortunately, is_nested is some sort of custom property, which means we have to break it before we can implement DataType.is_nested() as a classmethod.

Changes

  • Add the NESTED_DTYPES group
  • Deprecate DataType.is_nested, suggest using dtype in pl.NESTED_DTYPES instead (until we implement DataType.is_nested() as a breaking change).

@alexander-beedie Any thoughts?

@github-actions github-actions bot added deprecation Add a deprecation warning to outdated functionality python Related to Python Polars labels Oct 19, 2023
@stinodego stinodego marked this pull request as ready for review October 19, 2023 09:06
@alexander-beedie
Copy link
Collaborator

alexander-beedie commented Oct 20, 2023

The changes here all make sense to me, though I'm not quite as convinced about having a method per-dtype (in the future). Though maybe it's just... extremely clear? 🤔

Could also have a series of more generic methods so we don't have to update the class every time we add a new dtype (which isn't going to be often, but still... Thinking out loud, something along these lines might also work?

General match, eg: "Datetime == Datetime("us")
Exact match, eg: "Datetime("us") == Datetime("us"):

  • dtype.is_( ..., exact=True/False )
  • dtype.is_not( ..., exact=True/False )

Matches one of the given types:

  • dtype.in_( ..., exact=True/False )
  • dtype.not_in( ..., exact=True/False )

Need to see a few proposed usage examples to make better sense of it; perhaps we want to have dtype-specific and generic methods?


This also reminds me that I still need to get around to adding support for wildcard selection of nested/compound types in col (for selectors) 😅

@stinodego
Copy link
Member Author

I was planning on opening an issue explaining my intentions here, will do so shortly! Then we can take some time to discuss before I dive in 😄

Whichever way we go, though, I think we need to get rid of the custom is_struct property ASAP as I believe it should have been a classmethod in the first place and it cannot neatly be replaced without being a breaking change.

@alexander-beedie
Copy link
Collaborator

Whichever way we go, though, I think we need to get rid of the custom is_struct property ASAP as I believe it should have been a classmethod in the first place and it cannot neatly be replaced without being a breaking change.

Agreed! :)

@stinodego stinodego merged commit d9f2f5f into main Oct 20, 2023
14 checks passed
@stinodego stinodego deleted the depr-is-nested branch October 20, 2023 09:20
Jaxs88

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation Add a deprecation warning to outdated functionality python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants