-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
add/delete str/dt/cat dynamically from __dir__ (fix for #9627) #9910
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
Conversation
rv = set() | ||
if not is_datetimelike(self): | ||
rv.add('dt') | ||
if not com.is_categorical_dtype(self.dtype): |
There was a problem hiding this comment.
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
|
||
def _dir_deletions(self): | ||
try: | ||
self._check_str_accessor() |
There was a problem hiding this comment.
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:
# ...
There was a problem hiding this comment.
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
Some thoughts on the design here: we could add a class attribute def _dir_deletions(self):
deletions = set()
for accessor in self._possible_accessors:
try:
getattr(self, accessor)
except AttributeError:
deletions.add(accessor)
return deletions |
@shoyer I like this idea! I noticed that there's already a
but it's not obvious to me how this is being used. |
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 |
Yes, looks like |
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__()) |
There was a problem hiding this comment.
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.
LGTM, though you might test the built-in function |
@shoyer that makes sense, it's updated |
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. |
travis is green now yeah I hope the IPython auto-completion will work soon |
add/delete str/dt/cat dynamically from __dir__ (fix for #9627)
thanks! |
great news - the issue has been closed on the IPython side, we should soon have the tab completion working |
@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.