Metadata handling updates #53
Conversation
| @@ -64,7 +64,8 @@ | |||
| #----------------------------------------------------------------------------- | |||
| # Basic classes | |||
| #----------------------------------------------------------------------------- | |||
|
|
|||
| import warnings | |||
| warnings.filterwarnings('always') | |||
There was a problem hiding this comment.
Remove these in the final PR
jasongrout
force-pushed
the
metadata
branch
6 times, most recently
from
a79ed5f
to
184b694
Compare
jasongrout
changed the title
|
I think this is ready for review. |
|
Great, thanks! On Wed, Jul 22, 2015 at 12:25 PM, Jason Grout notifications@github.com
Brian E. Granger |
|
You emacs seem to make the test fail. |
|
hehe...couldn't be emacs, must be human error :) |
|
All the above PRs to the various repos are works-in-progress. |
|
Yep, you're right. I was over-eager in my replacements early on, including here. Edit: Fixed, rebased, and pushed. |
|
Actually, we should probably make help a first-class argument of the constructor on this PR too. So this is still WIP. |
…he metadata attribute As a consequence, deprecate the get_metadata and set_metadata functions.
The promotion will happen in a different PR, but for now, we'll not trigger the deprecation message for setting metadata when specifying just the help argument.
|
rebased on master... |
| @@ -359,13 +358,21 @@ def __init__(self, default_value=Undefined, allow_none=None, read_only=None, **m | |||
| ) | |||
|
|
|||
| if len(metadata) > 0: | |||
| warn("metadata should be set using the .tag() method, e.g., Int().tag(key1='value1', key2='value2')", DeprecationWarning, stacklevel=2) | |||
There was a problem hiding this comment.
This should include the metadata keys being set. Most people who encounter this message won't have any idea what values are metadata and what aren't.
|
We'll do the help promotion in another PR. It's sufficiently invasive and different that it deserves its own discussion. |
|
Okay, this is ready again for review. |
| @@ -368,7 +368,7 @@ class SomeSingleton(SingletonConfigurable): | |||
| pass | |||
|
|
|||
| class DefaultConfigurable(Configurable): | |||
| a = Integer(config=True) | |||
| a = Integer().tag(config=True) | |||
| def _config_default(self): | |||
| if SomeSingleton.initialized(): | |||
| return SomeSingleton.instance().config | |||
There was a problem hiding this comment.
Can we keep some tests that pass config=True in the constructor to ensure we are preserving the deprecated behavior? I know it's tested at the lower level in traitlets, but I'd like to keep one or two here, as well.
There was a problem hiding this comment.
Sure. Which test(s) do you want? I'll copy it and check for the deprecation warning too.
There was a problem hiding this comment.
Just one or two of the simplest existing tests. All I really care about is keeping track of a = Trait(config=True) still working and loading its value from the config object.
There was a problem hiding this comment.
Okay, then jasongrout@eab41d6 should be sufficient (i.e., the commit below).
An earlier commit accidentally introduced a second TestTraitType class that overrode the first, so about 10 tests weren't being run.
Add .tag method for adding metadata deprecate metadata as `**kwargs` in constructor
|
Will this be released in the next traitlet minor? Trying to decide when to merge Jason's widget PR - and for what version of widgets. |
|
I guess that we can require the next minor traitlets release in ipywidgets as soon as it is out. |
This PR implements the changes from #52.
TODO:
helpargument