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

add/delete str/dt/cat dynamically from __dir__ (fix for #9627) #9910

Merged
merged 1 commit into from
Apr 21, 2015

Conversation

mortada
Copy link
Contributor

@mortada mortada commented Apr 15, 2015

@jreback this PR makes it easier to add/delete items from __dir__. The .str/.dt/.cat are now only present in __dir__ when they are appropriate types.

However, the IPython tab completion does not seem to only source the list from __dir__, and therefore even if an item is removed from __dir__ it is still showing up in tab completion for me.

@shoyer I'd probably need some help/feedback with this. Here's a related ticket #9617

Thanks.

rv = set()
if not is_datetimelike(self):
rv.add('dt')
if not com.is_categorical_dtype(self.dtype):
Copy link
Contributor

Choose a reason for hiding this comment

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

all of these accessors (cat,dt,str) are mutually exclusive, so should set in a single routine. IOW, maybe change delete all, then add back in an if-then

@jreback jreback added the Output-Formatting __repr__ of pandas objects, to_string label Apr 17, 2015
@jreback jreback added this to the 0.16.1 milestone Apr 17, 2015

def _dir_deletions(self):
try:
self._check_str_accessor()
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to actually make a separate method _check_str_accessor(). You could simply do:

try:
    self.str
except AttributeError:
    # ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did that to avoid having to instantiate StringMethods(self) when it is not needed, but I agree it's probably premature optimization

@shoyer
Copy link
Member

shoyer commented Apr 17, 2015

Some thoughts on the design here: we could add a class attribute _possible_accessors which corresponds to a set of all possibly accessors (e.g., Series._possible_acccessors = set(['cat', 'str', dt'])). Then we can have something like the following generic method defined on PandasObject:

def _dir_deletions(self):
    deletions = set()
    for accessor in self._possible_accessors:
        try:
            getattr(self, accessor)
        except AttributeError:
            deletions.add(accessor)
    return deletions

@mortada
Copy link
Contributor Author

mortada commented Apr 17, 2015

@shoyer I like this idea! I noticed that there's already a _accessors attribute under Series though

_accessors = frozenset(['dt', 'cat', 'str'])

but it's not obvious to me how this is being used.

@shoyer
Copy link
Member

shoyer commented Apr 17, 2015

With regards to IPython auto-completion, this does seem trickier than I thought. Take a look at this example:

class A(object):
    something = 'asdf'

    def __dir__(self):
        return ['other']


class B(object):
    def __init__(self):
        self.something = 'asdf'

    def __dir__(self):
        return ['other']

a = A()
b = B()

Autocomplete on a still picks up something, even though we excluded it from __dir__ (on b, it's excluded). I'm guessing IPython uses dir(A) somehow.... might be worth asking over there, since I'm pretty puzzled at this point :).

@shoyer
Copy link
Member

shoyer commented Apr 17, 2015

Yes, looks like _accessors is probably the appropriate attribute to use then :). You'll have to do some searching to figure out how it's currently used.

@mortada
Copy link
Contributor Author

mortada commented Apr 20, 2015

@shoyer updated to incorporate your suggestions, now using self._accessors
@jreback updated to make use of the fact that .str/.dt/.cat are mutually exclusive

@jreback
Copy link
Contributor

jreback commented Apr 20, 2015

lgtm

pls add a release note (use the pr number as the issue)

@@ -1232,6 +1232,13 @@ def test_str_attribute(self):
expected = Series(range(2), index=['a1', 'a2'])
tm.assert_series_equal(s[s.index.str.startswith('a')], expected)

def test_tab_completion(self):
idx = Index(list('abcd'))
self.assertTrue('str' in idx.__dir__())
Copy link
Member

Choose a reason for hiding this comment

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

you can equivalently use the built-in function dir here instead.

@shoyer
Copy link
Member

shoyer commented Apr 20, 2015

LGTM, though you might test the built-in function dir directly instead of using __dir__

@mortada
Copy link
Contributor Author

mortada commented Apr 20, 2015

@shoyer that makes sense, it's updated

@shoyer
Copy link
Member

shoyer commented Apr 20, 2015

OK, looks good to me! Please ping when it Travis is green.

Too bad this won't work for IPython auto-complete yet... but at least they're working on it.

@mortada
Copy link
Contributor Author

mortada commented Apr 21, 2015

travis is green now

yeah I hope the IPython auto-completion will work soon

shoyer added a commit that referenced this pull request Apr 21, 2015
add/delete str/dt/cat dynamically from __dir__ (fix for #9627)
@shoyer shoyer merged commit 6908719 into pandas-dev:master Apr 21, 2015
@shoyer
Copy link
Member

shoyer commented Apr 21, 2015

thanks!

@mortada mortada deleted the tab_completion branch April 21, 2015 01:04
@mortada
Copy link
Contributor Author

mortada commented Apr 21, 2015

great news - the issue has been closed on the IPython side, we should soon have the tab completion working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Output-Formatting __repr__ of pandas objects, to_string
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants