Skip to content

Conversation

@dsaxton
Copy link
Contributor

@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