8000 Implement _make_accessor classmethod for PandasDelegate by jbrockmendel · Pull Request #17166 · pandas-dev/pandas · GitHub
[go: up one dir, main page]
Skip to content

Implement _make_accessor classmethod for PandasDelegate #17166

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 8, 2017
Merged

Implement _make_accessor classmethod for PandasDelegate #17166

merged 2 commits into from
Aug 8, 2017

Conversation

jbrockmendel
Copy link
Member

This is an absolutely minimal subset of the refactor in #17042. It should be obvious and uncontroversial.

  • Define _make_accessor as a classmethod of PandasDelegate.

  • Make AccessorProperty.__init__ argument construct_accessor default to accessor_cls._make_accessor.

  • Remove methods _make_cat_accessor and _make_dt_accessor from Series, as they do not belong in the namespace. Remove _make_str_accessor from both Series and Index namespace.

  • closes #xxxx

  • tests added / passed

  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

  • whatsnew entry

@gfyoung gfyoung added Clean Internals Related to non-user accessible pandas implementation labels Aug 3, 2017
gfyoung
gfyoung reviewed Aug 3, 2017
View reviewed changes
pandas/core/base.py Outdated
def _make_accessor(cls, data):
raise NotImplementedError("_make_accessor should be implemented"
"by subclass. It should return an instance"
"of `cls`.")
Copy link
Member

Choose a reason for hiding this comment

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

Use instead: "_make_accessor should be implemented by subclass and return an instance of cls."

Copy link
Contributor

Choose a reason for hiding this comment

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

we have AbstracMethodError for this

gfyoung
gfyoung reviewed Aug 3, 2017
View reviewed changes
pandas/core/indexes/accessors.py Outdated
return maybe_to_datetimelike(data)
except Exception:
raise AttributeError("Can only use .dt accessor with datetimelike "
"values")
Copy link
Member

Choose a reason for hiding this comment

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

Nit: can we re-organize so that we don't have a lone word on this line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. I'll take care of this and anything else that piles up this evening. To the extent that we can exercise self-control, I'm fervently hoping to keep the nitpicking back in #17042.

@codecov
Copy link
codecov bot commented Aug 3, 2017 •
edited
Loading

Codecov Report

Merging #17166 into master will increase coverage by <.01%.
The diff coverage is 95.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17166      +/-   ##
==========================================
+ Coverage   90.99%      91%   +<.01%     
==========================================
  Files         162      162              
  Lines       49499    49502       +3     
==========================================
+ Hits        45043    45048       +5     
+ Misses       4456     4454       -2
Flag Coverage Δ
#multiple 88.78% <95.45%> (+0.02%) ⬆️
#single 40.26% <72.72%> (-0.06%) ⬇️
Impacted Files Coverage Δ
pandas/core/strings.py 98.48% <100%> (ø) ⬆️
pandas/core/series.py 94.99% <100%> (-0.05%) ⬇️
pandas/core/categorical.py 95.48% <100%> (+0.02%) ⬆️
pandas/core/indexes/accessors.py 88.76% <100%> (+0.66%) ⬆️
pandas/core/base.py 96% <75%> (-0.21%) ⬇️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.66% <0%> (-0.1%) ⬇️
pandas/core/internals.py 94.03% <0%> (ø) ⬆️
pandas/plotting/_converter.py 65.05% <0%> (+1.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c4bc05...9c0ae3f. Read the comment docs.

jreback
jreback approved these changes Aug 3, 2017
View reviewed changes
Copy link
Contributor
@jreback jreback left a comment

Choose a reason for hiding this comment

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

small nit, otherwise lgtm. ping on green.

pandas/core/base.py Outdated
def _make_accessor(cls, data):
raise NotImplementedError("_make_accessor should be implemented"
"by subclass. It should return an instance"
"of `cls`.")
Copy link
Contributor

Choose a reason for hiding this comment

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

we have AbstracMethodError for this

pandas/core/base.py Outdated
self.accessor_cls = accessor_cls
if construct_accessor is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

self.construct_accessor = construct_accessor or accessor_cls._make_accessor is idiomatic

@jreback jreback added this to the 0.21.0 milestone Aug 3, 2017
@jreback jreback merged commit 7bef6d8 into pandas-dev:master Aug 8, 2017
@jreback
Copy link
Contributor
jreback commented Aug 8, 2017

thanks!