Skip to content
This repository has been archived by the owner on Oct 15, 2019. It is now read-only.

[WIP] autocompletion #86

Merged
merged 2 commits into from
Dec 2, 2016
Merged

[WIP] autocompletion #86

merged 2 commits into from
Dec 2, 2016

Conversation

ZihengJiang
Copy link
Member

WIP

@ZihengJiang ZihengJiang changed the title [autocompletion] add __dir__ in Module [WIP] autocompletion Nov 30, 2016
@jermainewang
Copy link
Member

Can we directly use setattr in the init function to directly put them in the module object? I think this will save us another indirect call and it will automatically be added to dir.

@jermainewang
Copy link
Member

@ZihengJiang
Copy link
Member Author

good idea! @jermainewang

for fname in self._name_injector.keys():
setattr(self, fname, self._name_injector[fname])
for fname in self._registry._reg.keys():
fun = PrimitiveSelector(fname, self._registry, self._policy)
Copy link
Member

Choose a reason for hiding this comment

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

Does auto-completion require function signature? If so, our PrimitiveSelector will break it.

@jermainewang
Copy link
Member

LGTM

@ZihengJiang ZihengJiang merged commit cc3e770 into master Dec 2, 2016
@lryta
Copy link
Member

lryta commented Dec 3, 2016

This PR fails Travis check.

@ZihengJiang
Copy link
Member Author

Could you paste the Travis log link?

@lryta
Copy link
Member

lryta commented Dec 3, 2016

You can check the status of Travis. I fired a patch.

@ZihengJiang
Copy link
Member Author

alright, I see, due to this method make the attributes fixed when initialize modules, but sometimes we need to change policy during runtime. Thank you!

@ZihengJiang
Copy link
Member Author

I think this problem can be addressed by two different ways:

  • revert this pr, return attributes by defined __dir__, every time change the policy should change the keys the __dir__ returns.
  • continue using setattrs, but every time change the policy should reset modules' attributes

what's your opinion? @jermainewang @lryta

@jermainewang
Copy link
Member

Could you completely disallow changing policies in the runtime? I guess this is what we want to push?

@ZihengJiang
Copy link
Member Author

sounds good

@jermainewang jermainewang mentioned this pull request Dec 3, 2016
@jermainewang
Copy link
Member

Let's move the discussion to #93

@jermainewang jermainewang deleted the autocompletion branch December 3, 2016 17:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants