Skip to content

REF/CLN: Parametrize _isna #33835

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

Merged
merged 5 commits into from
Apr 29, 2020
Merged

REF/CLN: Parametrize _isna #33835

merged 5 commits into from
Apr 29, 2020

Conversation

dsaxton
Copy link
Member

@dsaxton dsaxton commented Apr 27, 2020

Replaces two nearly identical isna functions with one by parametrizing over the difference

@dsaxton dsaxton added Clean Refactor Internal refactoring of code labels Apr 28, 2020
@jbrockmendel
Copy link
Member

LGTM.

_isna_ndarraylike should get a similar treatment; IIRC there is an issue with EAs not handling old correctly

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @dsaxton generally lgtm



def _isna_old(obj):
def _isna(obj, old: bool = False):
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to use, say, inf_as_na = True/False moving forward and remove the old concept for clarity. (follow-on ok too)

also could maybe use lib.no_default and look up options if not passed instead of the using the callback and partial.

Copy link
Contributor

Choose a reason for hiding this comment

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

i agree this makes it more clear what is going on

@simonjayhawkins simonjayhawkins added this to the 1.1 milestone Apr 28, 2020


def _isna_old(obj):
def _isna(obj, old: bool = False):
Copy link
Contributor

Choose a reason for hiding this comment

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

i agree this makes it more clear what is going on

@jorisvandenbossche jorisvandenbossche merged commit 4d0f179 into pandas-dev:master Apr 29, 2020
@jorisvandenbossche
Copy link
Member

Thanks!

@dsaxton dsaxton deleted the refactor-isna branch April 29, 2020 12:38
jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Apr 29, 2020
rhshadrach pushed a commit to rhshadrach/pandas that referenced this pull request May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants