ENH: Added public accessor registrar
State: closed

Adds new methods for registing custom accessors to pandas objects.

This will be helpful for implementing https://github.com/pandas-dev/pandas/issues/18767
outside of pandas.

If we accept this I'm not sure it belongs in the top-level namespace where I have it currently. What would a good home be for this be? `pd.api.extensions`?

Closes https://github.com/pandas-dev/pandas/issues/14781

Labels:
name: Enhancement
name: API Design

Comments:
Hello @TomAugspurger! Thanks for updating the PR.





























Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on January 15, 2018 at 12:34 Hours UTC
@TomAugspurger : A couple of points:

1) Re: your question, it depends on how much importance you put on the resource.  Personally, I'm fine with exposing it in the top-level namespace.  `xarray` does this, so one could argue that we should do the same for consistency.  Unless you're using this solely for internal stuff, then I don't see why not.

2) Could you explain the differences / changes you needed to make for this to port over to `pandas` ?  That would be very useful for understanding why you needed to add this to `pandas`.
There might be some useful things to pull out of the [tests](https://github.com/pandas-dev/pandas/pull/17042/files#diff-6df9fe81c485ca477e339301a31cc53b) for the now-abandoned #17042.  I wrote a moderately fleshed out example for that.

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18827?src=pr&el=h1) Report
> Merging [#18827](https://codecov.io/gh/pandas-dev/pandas/pull/18827?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/787ab5556f23d2e10b798d2e58e66789d79fd4d5?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `93.91%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/18827/graphs/tree.svg?src=pr&width=650&token=eZ4WkYLtcO&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/18827?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #18827      +/-   ##
==========================================
- Coverage   91.55%   91.53%   -0.02%     
==========================================
  Files         147      148       +1     
  Lines       48812    48837      +25     
==========================================
+ Hits        44690    44704      +14     
- Misses       4122     4133      +11
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.91% <93.91%> (-0.02%)` | :arrow_down: |
| #single | `41.7% <57.39%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18827?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.62% <100%> (ø)` | :arrow_up: |
| [pandas/api/extensions/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2FwaS9leHRlbnNpb25zL19faW5pdF9fLnB5) | `100% <100%> (ø)` | |
| [pandas/core/accessor.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYWNjZXNzb3IucHk=) | `98.7% <100%> (+4.95%)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `94.61% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.46% <100%> (ø)` | :arrow_up: |
| [pandas/errors/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2Vycm9ycy9fX2luaXRfXy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [pandas/core/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY2F0ZWdvcmljYWwucHk=) | `95.78% <100%> (ø)` | :arrow_up: |
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `96.77% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/accessors.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9hY2Nlc3NvcnMucHk=) | `90% <88%> (+0.63%)` | :arrow_up: |
| [pandas/core/strings.py](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3RyaW5ncy5weQ==) | `98.17% <92.85%> (-0.3%)` | :arrow_down: |
| ... and [2 more](https://codecov.io/gh/pandas-dev/pandas/pull/18827/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18827?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18827?src=pr&el=footer). Last update [787ab55...fd40244](https://codecov.io/gh/pandas-dev/pandas/pull/18827?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

this should not be in the main public namespace. 
Here's the relevant diff between xarray and pandas:

```diff
diff --git a/xarray.py b/pandas.py
index 48b2cba..fc3dd6b 100644
--- a/xarray.py
+++ b/pandas.py
@@ -11,9 +11,10 @@ class _CachedAccessor(object):
         try:
             accessor_obj = self._accessor(obj)
         except AttributeError:
-            # __getattr__ on data object will swallow any AttributeErrors raised
-            # when initializing the accessor, so we need to raise as something
-            # else (GH933):
+            # TODO
+            # __getattr__ on data object will swallow any AttributeErrors
+            # raised when initializing the accessor, so we need to raise
+            # as something else (GH933):
             msg = 'error initializing %r accessor.' % self._name
             if PY2:
                 msg += ' Full traceback:\n' + traceback.format_exc()
@@ -30,9 +31,9 @@ def _register_accessor(name, cls):
     def decorator(accessor):
         if hasattr(cls, name):
             warnings.warn(
-                'registration of accessor %r under name %r for type %r is '
-                'overriding a preexisting attribute with the same name.'
-                % (accessor, name, cls),
+                'registration of accessor {!r} under name {!r} for type '
+                '{!r} is overriding a preexisting attribute with the same '
+                'name.'.format(accessor, name, cls),
                 AccessorRegistrationWarning,
                 stacklevel=2)
         setattr(cls, name, _CachedAccessor(name, accessor))

```

(my TODO is for whether or not that matters for pandas). Otherwise, we're trying to use new-style string formatting.
Updated.

1. xarray had to catch `AttributeError` raised when creating the accessor due to how their `__getitem__` works. I don't think pandas needs to do this (https://github.com/pydata/xarray/issues/933)
2. The extra level for `pd.api.extensions` rather than `pd.extensions` seems unnecessary (flat is better than nested and all that), but OK either way
3. `pytest.monkeypatch` seems to be for functions, not instances of objects, unless I'm mistaken
4. I have a slight preference to focusing this PR on just adding the `register_*_accessor`, but I can also refactor the internals to use it. Really though, these are independent. The only real change would be to `pd.api.extensions.resgister_series_accessor("str")(StringMethods)` instead of `str = accessor.AccessorProperty(strings.StringMethods)`. The `register*` decorators are just for registering, not for implementing accessors themselves.
> I have a slight preference to focusing this PR on just adding the register_*_accessor, but I can also refactor the internals to use it. Really though, these are independent. The only real change would be to pd.api.extensions.resgister_series_accessor("str")(StringMethods) instead of str = accessor.AccessorProperty(strings.StringMethods). The register* decorators are just for registering, not for implementing accessors themselves.

I disagree. The best use of this is to actually implement the internals. If it works, great, if not then you have found a bug / feature lacking.
Refactored to have our accessors use it.

1.  All the accessors are registered in `pandas.core.register_accessors` after the classes have been initialized
2. I ran into a subtle issue around caching. The accessor from xarray is cached such that `x.foo` is the same object everytime. With our old ``AccessorProperty``, we recreated the accessor every time. This caused some issues when the object itself was mutable, but the accessor cached some state. Notably, this would occur:

```python
In [9]: s = pd.Series(pd.TimedeltaIndex(['5S', '10S']))

In [10]: s.dt.values.isna()
Out[10]: array([False, False], dtype=bool)

In [11]: s[0] = float('nan')

# isna has been cached on the accessor's .values, but not invalidated
# when s was mutated
In [12]: s.dt.values.isna()
Out[12]: array([ False, False], dtype=bool)
```

To work around this, I added a `cache` keyword argument (@shoyer would xarray be interested in this keyword?)

3. Moved the new docs to "developer.rst" instead of internals, since it's for "downstream applications" (we should probably move the subclassing docs there)
4. Removed the now unused AccessorProperty class
> I ran into a subtle issue around caching. The accessor from xarray is cached such that x.foo is the same object everytime. With our old AccessorProperty, we recreated the accessor every time. This caused some issues when the object itself was mutable, but the accessor cached some state. 
>
> To work around this, I added a cache keyword argument (@shoyer would xarray be interested in this keyword?)

This seems like an inadvertent detail of the implementation of `Series.dt.values` (why do we even have that property?). The datetime accessor creates a DatetimeIndex internally for method/properties. `DatetimeIndex.values` is fixed, because pandas.Index objects are immutable.

So the other fix would be to change how the `.dt` accessor implemented so that `DatetimeIndex` objects are created on demand when a method/property is called, instead of being created once and saved on the acccessor.

I don't think we need a `cache=False` option for xarray since nobody has written any accessors relying on this behavior.
Making it easy to extend functionality is great, but the new commit that changes where the current str/cat/dt attributes are defined I'm very much -1 on.  Inheritance notwithstanding, the class definition should be WYSIWYG.  Newbies shouldn't have to [track down](https://github.com/pandas-dev/pandas/pull/17174/files#diff-de56e43aab6f9b0909c90246728a3662L207) where e.g. `DataFrame.pivot_table` is defined.
> This seems like an inadvertent detail of the implementation of Series.dt.values (why do we even have that property?).

`.values` isn't actually in `dir(s.dt)`. Should probably be `._values` but that's how it was before. Regardless, I'll see how difficult it is to create them on demand.

> the class definition should be WYSIWYG

That's my preference as well, we just won't be able to / have to use the `register_*_accessor` functions, since those rely on the classes being defined.
@jreback @jbrockmendel thoughts on the refactored version? Anything else you'd like to see changed?
> thoughts on the refactored version?

Completely addresses my WYSIWYG complaint, thanks.
>  this is too general we need to be opionated, if there is no validation then that should be explicit.

@jreback could you clarify your thoughts here? I've documented that the only requirement on the user-defined accessor is that it's `__init__` will be called with `pandas_obj`. I don't think there's anything the `_CachedAccessor` can / should do w.r.t. validation. With an accessor like `.plot` there's nothing to be done, so it doesn't make sense for accessors to implement a method that does nothing.