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

make_block compat #19444

Closed
jbrockmendel opened this issue Jan 29, 2018 · 1 comment
Closed

make_block compat #19444

jbrockmendel opened this issue Jan 29, 2018 · 1 comment
Labels

Comments

@jbrockmendel
Copy link
Member

jbrockmendel commented Jan 29, 2018

Related: #19431, #9859, #19294

We've been making progress on getting Index and Series arithmetic&comparison methods to behave identically by having the Series versions wrap the Index implementations. To also make DataFrame behavior consistent, we're going to need to move those implementations from the Index subclasses to a shared mixin that will be mixed in to both the Index and Block subclasses (e.g. DatetimeIndex and DatetimeBlock both subclass DatetimeArray cc: @TomAugspurger).

To get from here to there, we need to standardize some some behavior and naming conventions that don't quite match between Index and Block. The analogous methods AFAICT are:

  • Index.__new__ <--> internals.make_block
  • Index._shallow_copy_with_infer <--> Block.make_block
  • Index._shallow_copy <--> Block.make_block_same_class
  • Index._simple_new <--> [needs implementation]

There is an inconsistency between Block.make_block and Block.make_block_same_class that needs to be resolved before these analogies become just a matter of naming convention. Block.make_block sets default values for the kwargs placement, ndim, while make_block_same_class only chooses a default for placement. ndim needs to be either added to make_block_same_class or removed from make_block in order for _get_attributes_dict to be definable.

@jreback jreback added the Clean label Feb 1, 2018
@jreback
Copy link
Contributor

jreback commented Feb 1, 2018

maybe. this is conflating different apis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants