-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Do not instantiate traitlet default values if not necessary #7770
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
|
Ok, Travis is happy now - ready for review. |
4a4b609 to
5d2961a
Compare
|
Rebased to account for the latest changes merged in traitlets, and then simplified a little bit. |
ea2ca03 to
2837617
Compare
|
Just rebased but it seems to make Travis unhappy. |
|
The failing test seems unrelated. |
|
Can you document the new functionality? The methods you replaced were public APIs whose behavior was documented. |
|
Yes, I will add docstrings this afternoon. In short
The base trait type has an empty Now, the goal is to deffer the execution of |
2837617 to
2a4821c
Compare
|
Great, thanks for the explanation. I think |
|
Done, I just added some documentation and renamed the methods accordingly to your recommendation. Indeed decorate is quite overloaded in Python. |
|
Terrific, thanks. I think it's a lot easier to follow, now. Will merge when Travis gives the green light. |
bcbb3a0 to
4da3271
Compare
|
The failing tests are real, but due to a slightly pathological behavior in the in-process kernel init. The in-process kernel asks for traits passed in kwargs before calling A test case: from IPython.utils.traitlets import HasTraits, Instance
class C(HasTraits):
t = Instance(int, allow_none=False)
def _t_default(self):
return 5
def __init__(self, **kwargs):
print(self.t)
super().__init__(self, **kwargs)
C() # works, t=5
C(t=2) # Fails with t is None, but allow_none=False
I don't think it's a problem that you cannot reliably access dynamic trait initialization before calling |
appears to be unnecessary due to upstream changes, and no longer works with traitlets refactor.
|
I made SylvainCorlay#3 to remove the weird in-process behavior. |
|
Thanks, I was just looking again into this. (the tests used to pass before I rebased.) |
remove problematic in-process kernel behavior
Do not instantiate traitlet default values if not necessary
|
Thanks! |
Fixes #7769 .
This is not very polished but seems to be doing the job. Will not be able to look more into it before the week end.