Comments (33)
👍 for either explicit metadata
as a dictionary argument, or for .tag
accepting **kwargs
from traitlets.
+1 on A1 and B1
On Fri, Jul 17, 2015 at 8:59 AM, Scott Sanderson [email protected]
wrote:
[image: 👍] for either explicit metadata as a dictionary argument, or
for .tag accepting **kwargs—
Reply to this email directly or view it on GitHub
#52 (comment).
Brian E. Granger
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
[email protected] and [email protected]
from traitlets.
Splitting my vote, +0.7 on option A1, and +0.3 on B3. A1 is more pythonic. Python does not generally chain methods onto constructors, and usually methods that mutate state (like tag
does in this case) would return None, not the object. The thing I don't like about A1 is that it is the most verbose of all the options, which is almost enough for me to vote for B3.
from traitlets.
There might also be an option A3 using tags
to shorten the name but still keeping the explicit argument:
x = Int(allow_none=True, tags={'sync': True})
from traitlets.
B1 or B3
from traitlets.
The reason that atom is using tag
is because there is a metadata
read-write property on the member instance. tag
is just a shortcut to set metadata
and return self
. I'm not a fan of a metadata
keyword argument.
from traitlets.
@ellisonbg - do you know why traitlets has a metadata class member that is used as the default metadata, but the real metadata is stored in ._metadata
with get_metadata
and set_metadata
functions? I've been wondering why we don't just have a metadata attribute that is a dict, with some other way of having default metadata specified in the class.
from traitlets.
Can we consider making a project with a new name for changes like this rather than changing the API of traitlets? I don't relish the idea of having to go through and change most of the files in a dozen or so projects to a revised traitlets API. And starting to plan backwards-incompatible API changes straight after the first release doesn't exactly inspire confidence that it will be a stable API for other people to build on.
from traitlets.
@takluyver, we have the manpower to change the dozens of files (I just did it for the Int vs. Int() change for another PR). These changes we are discussing are backwards-compatible, with deprecations that will probably be measured in years (at the rate we release things, anyway). What we're wanting to do is provide ways around the warts in traitlets for new code, particularly just before/while other projects are starting to adopt the traitlets library, so that they can write cleaner code.
from traitlets.
@jasongrout I don't remember why I implemented metadata that way :(
@takluyver trying to evolve software projects and never have API breakage is simply not realistic for projects that are on the edge of a rapidly changing ecosystem. This is what version numbering is for. I think we should absolutely have deprecation cycles and make this easy on users, but to create forks of our own projects just to a avoid API changes doesn't make sense as most people will switch over to the new, faster moving project. In the end, the same thing can be accomplished with good version numbers.
from traitlets.
I would prefer to stick with the historic name metadata
in traitlets, even as we evolve the API.
from traitlets.
Option B1 conflicts with the current .metadata
attribute that provides default class-level metadata, and it's the natural name for the actual metadata attribute, so -1 on Option B1.
from traitlets.
Of the B options, I like B3 the best, since it's short and it's a verb. Another good thing about .tag()
is that it doesn't convey that it is the only metadata. Having metadata as a constructor argument seems to indicate that what is passed is the only metadata the trait will have, but in reality we'd be combining (and overriding) the default metadata. I think .tag()
more clearly conveys that we are updating the existing metadata dictionary.
from traitlets.
As for the default metadata, how about this change in behavior (assuming we go with option A1, but similarly for other options):
class MyTraitedClass(HasTraits):
metadata = {'key': 'value'} # default class-level metadata
def __init__(self, ..., metadata=None):
self.metadata = self.metadata.copy()
if metadata is not None:
self.metadata.update(metadata)
It would make metadata retrieval much simpler, and also preserve the use of the class member for default metadata. I don't like that it changes the purpose of the .metadata
attribute when the class is instantiated by localizing the member, but it would make the change backwards-compatible for probably all real-world use-cases. Of course, we'd deprecate the get_metadata()
and set_metadata()
functions, preferring direct operations with the metadata dict.
Edit: fixed the update to correctly override defaults.
from traitlets.
i am fine with this idea for handling default metadata...
On Fri, Jul 17, 2015 at 10:27 AM, Jason Grout [email protected]
wrote:
As for the default metadata, how about this change in behavior (assuming
we go with option A1, but similarly for other options):class MyTraitedClass(HasTraits):
metadata = {'key': 'value'} # default class-level metadata
def init(self, ..., metadata=None):
if metadata is None:
metadata = {}
metadata.update(self.metadata)
self.metadata = metadataIt would make metadata retrieval much simpler, and also preserve the use
of the class member for default metadata. I don't like that it changes the
purpose of the .metadata attribute when the class is instantiated by
localizing the member, but it would make the change backwards-compatible
for probably all real-world use-cases. Of course, we'd deprecate the
get_metadata() and set_metadata() functions, preferring direct operations
with the metadata dict.—
Reply to this email directly or view it on GitHub
#52 (comment).
Brian E. Granger
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
[email protected] and [email protected]
from traitlets.
@jasongrout I think you have your update
arguments reversed. The way you have it, the default metadata would override the user supplied metadata.
from traitlets.
@sccolbert - right! Fixed.
from traitlets.
OK given the conflict with the already existing metadata attribute in B1, I vote for B3.
And I would -1 the choice of having a metadata
dict keyword argument. It is too long for things such as specifying sync=True
in widgets...
from traitlets.
Any backward-incompatible changes here are going to have to go through a long (at least a year or two) deprecation cycle.
from traitlets.
@minrk - yep. I think an exception could be made for making the class-level metadata dict a local one when an instance is created. I'd probably consider that change more of a bugfix than a deliberate change in intended behavior (I sure hope no one was changing class-level metadata defaults by changing the metadata attribute on an instance!).
from traitlets.
@jasongrout sure, hidden implementation details are different than the public API. I think an important piece of this is not changing the existing tests that exercise the deprecated patterns, but adding new ones, instead. The existing tests need to keep passing.
from traitlets.
Yes, I think that's important too, except for changing the existing tests to check for deprecation messages too.
from traitlets.
Yup. It's something we are trying to do in the split that we haven't done well in the past - clearer communication of deprecations, and less abrupt transitions to new things across multiple releases. Preserving tests of deprecated APIs is part of keeping us honest.
from traitlets.
Seems like this discussion has stalled without a clear consensus.
- We want to improve this API to remove the problem of mixing arguments of the constructor with metadata key/values.
- We need to have a good long deprecation period of the old API.
- It is hard to evolve the API using the name
metadata
without breaking BW compat. - Having the new API use the name
tag
is one way of solving that. - There are a range of ideas on how to improve the API given these constraints.
I propose that all interested parties hop on a G+ to make the decision on Monday morning (let's say 9am PDT) if we can't find consensus before then.
from traitlets.
@ellisonbg sounds good. Do you want to create a hangout link for the meeting and post it here?
from traitlets.
sounds good.
from traitlets.
I hate G+, but I think I have done this properly:
https://plus.google.com/events/co224tq70q2ksdu8rfir6pjveq0
On Sat, Jul 18, 2015 at 10:04 AM, Sylvain Corlay [email protected]
wrote:
sounds good.
—
Reply to this email directly or view it on GitHub
#52 (comment).
Brian E. Granger
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
[email protected] and [email protected]
from traitlets.
Hi guys, I have created a section on our meeting hackpad for this meeting:
https://ipython.hackpad.com/2015-Dev-Meetings-Part-1-3YJG5lv2Hws
I think we should limit the meeting to an hour, but we may have time to make decisions on other traitlets API design things. @jasongrout and @SylvainCorlay can you put a list of the decisions you would like us to make at the meeting on the hackpad.
@fperez if you can't make it, can you appoint a BDFL delegate (maybe Min?) to break any lack of clear consensus?
from traitlets.
Unfortunately I can't make it, I have to be in SF all day for a meeting...
Indeed, I think @minrk is a perfect BDFL delegate in this case; Min has an extremely good knowledge of how the traitlets code impacts the config machinery throughout our entire codebase, including the management of clusters in IPython.parallel
.
Are you OK with that role, Min?
from traitlets.
Sure, I can do that.
from traitlets.
Excellent, thanks!
from traitlets.
@jasongrout Should we close this one?
from traitlets.
This was fixed in #53 and #56.
from traitlets.
Related Issues (20)
- docs do not mention if loading application data from config files is a security risk or not
- Should we open a discussions tab in this github?
- Union trait does not parse correctly from strings
- `TraitType.make_dynamic_default` is not mentioned in the docs
- Missing project description at PyPI HOT 1
- Version 5.7.0 raises AttributeError: __provides__ HOT 1
- `DeprecationWarning:` in traitlets HOT 5
- is it normal that SequenceTypes are not iterable ? HOT 1
- Bug in version 5.8.0 HOT 4
- `UseEnum` does not work with str enum
- How to create required CLI config options? HOT 5
- breaking change: __all__ limits what is exported
- instance_init of Union subclass not being called
- bug: use of warnings.warn() does not always include stacklevel
- Confused about self.parent HOT 1
- [error]You can use 'hmac-sha256' instead of '"hmac-sha256"' if you require traitlets >=5
- Application.load_config_file loads all files with same basename as given file HOT 3
- Clarification on the self.config object HOT 1
- Pyright diagnostic type incompatible
- Is there a way to only show base "--help" for explicitly defined options?
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from traitlets.