Skip to content

CLN: assorted cleanups #39856

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 10 commits into from
Feb 21, 2021
Merged

CLN: assorted cleanups #39856

merged 10 commits into from
Feb 21, 2021

Conversation

jbrockmendel
Copy link
Member

  • closes #xxxx
  • tests added / passed
  • Ensure all linting tests pass, see here for how to run them
  • whatsnew entry

Comment on lines 759 to 761
dtype = blocks[0].dtype
# TODO(EA2D): special case would be unnecessary with 2D EAs
return isinstance(dtype, np.dtype) # i.e. not ExtensionDtype
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
dtype = blocks[0].dtype
# TODO(EA2D): special case would be unnecessary with 2D EAs
return isinstance(dtype, np.dtype) # i.e. not ExtensionDtype
return not self._mgr.any_extension_type

(at this point you know there is only 1 block, so we are not checking it for potentially many columns as was the case before)

Copy link
Member Author

Choose a reason for hiding this comment

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

This paves the way for the DTBlock/TDBlock-backed-by-DTA/TDA PR, in which this isinstance check is only slightly tweaked

Copy link
Member

Choose a reason for hiding this comment

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

I suppose that those blocks should then still not be considered as "extension block" for any_extension_type, as the main reason that we use any_extension_type is to know if it has a non-consolidating / non-transposable block (and which would not be the case for DTBlock)

Copy link
Member Author

Choose a reason for hiding this comment

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

not worth arguing about, since only one line is in question and that line will get changed in the upcoming PR anyway. will use the proposed edit.

@jbrockmendel
Copy link
Member Author

any other issues here?

@jreback jreback added Clean Testing pandas testing functions or related to the test suite labels Feb 21, 2021
@jreback jreback added this to the 1.3 milestone Feb 21, 2021
@jreback jreback merged commit 95eee9d into pandas-dev:master Feb 21, 2021
@jbrockmendel jbrockmendel deleted the cln-2 branch February 21, 2021 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants