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

Fixes for traitlets 4.1 deprecation warnings #695

Merged
merged 1 commit into from Oct 31, 2017

Conversation

Projects
None yet
3 participants
@jcb91
Copy link
Contributor

jcb91 commented Oct 22, 2017

since 4.1, traitlets complains (issues DeprecationWarnings) about using TraitType classes to set the allowed trait types for container traits like Set and List. This PR switches to using instances, to avoid the warnings in the RegexRemovePreprocessor and ClearOutputPreprocessor classes.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 26, 2017

I don't actually see that documented anywhere and when I run these tests, I don't get those warnings…

Importantly, I fear that because it's not receiving a traittype anymore that it's not going to restrict the contents of the containers.

Could you actually paste the deprecation warnings you're running into?

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Oct 26, 2017

@mpacer this has been the right way to do it for some time, but there's some traitlets magic where if it sees a TraitType instead of TraitType instance, it will instantiate it automatically in the constructor. The implicit, automatic instantiation is what's deprecated. Behavior is unchanged.

To see the warning:

from traitlets import HasTraits, Unicode, List

class Klass(HasTraits):
    trait = List(Unicode)

c = Klass()

run with PYTHONWARNINGS=d gives:

$> PYTHONWARNINGS=d python tst.py
tst.py:4: DeprecationWarning: Traits should be given as instances, not types (for example, `Int()`, not `Int`). Passing types is deprecated in traitlets 4.1.
  trait = List(Unicode)
@minrk

minrk approved these changes Oct 26, 2017

@jcb91

This comment has been minimized.

Copy link
Contributor Author

jcb91 commented Oct 26, 2017

@minrk exactly, thanks for the clarification, apologies @mpacer, I should have given a better explanation. See traitlets/traitlets.py#L2443-L2448 for where the warnings originate from. I encountered them in nbconvert in the course of writing a warning checker for our nbextensions' nbconvert_support stuff ipython-contrib/jupyter_contrib_nbextensions#1137. I was a little surprised that I hadn't seen them before, but I guess it makes sense that the model is more opt-in-to-see than warn-everybody-all-the-time.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 27, 2017

Whats bizarre is pytest --PYTHONWARNINGS=all did not surface this. I didn't realise "all" didn't include deprecation warnings…

@mpacer mpacer merged commit 4abde40 into jupyter:master Oct 31, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mpacer mpacer added this to the 5.4 milestone Nov 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment