Skip to content
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

Re-export TraitList/TraitSet/TraitDict containers (not handlers) in API? #1332

Open
kitchoi opened this issue Oct 28, 2020 · 2 comments
Open

Comments

@kitchoi
Copy link
Contributor

kitchoi commented Oct 28, 2020

Currently TraitListObject, TraitSetObject and TraitDictObject are exposed in the api module, but their corresponding base classes TraitList, TraitSet and TraitDict are NOT exposed in the api module.

I am not sure whether TraitList are safe to be used externally, because of the following circumstantial information:
(1) Traits wants to impose this rule (see #1213): only guarantee backward compatibility for things that are exported via the api modules, and things that are not exported via the api modules do not receive backward compatibility treatment. -> so it is not safe to rely on it?
(2) The TraitList from trait_list_object has a name clash with the TraitList from trait_handlers. The latter is already exported in api, so understandably if I want to import the TraitList from trait_list_object, I cannot import it from api. -> so it is actually safe to rely on, but for technically difficulties it wasn't made obvious?
(3) The TraitList from trait_list_object is visible in the public API documentation. -> so it is actually safe to rely on? (conflicting with (1))

In any case, I find myself second guessing the intention due to the name clash with trait_handlers. If the TraitList (container) and friends are not meant to be subclassed externally, it might be good to rename them or document the intention in the docstring. If the TraitList is meant to be used externally, make it more obvious so that developers don't have to second guess if they are doing something not meant to happen.

(Note: Motivation for this issue is that I am trying to decide whether to subclass TraitList or TraitListObject for an issue in Envisage, after exhausting all other options that don't require subclass. Judging from my own understanding of TraitList and TraitListObject, I would think it is better to subclass TraitList, which knows itself is meant to be subclass-ed.)

@corranwebster
Copy link
Contributor

I'd like to eventually deprecate TraitListObject and friends (although not sure of the best path for now, and probably not for a major version or two) as they carry around a lot of baggage that they don't really need (such as being bound to a particular HasTraits instance). Similarly TraitList trait handlers are hopefully going to go away at the next major release as trait handlers have been deprecated.

I think that the TraitList etc. objects are reasonably safe to use externally - what they are trying to do is fairly constrained and complete and any further changes are likely to be along the lines of tracking the underlying container type APIs from Python itself. They were designed to be an eventual public API, and while it might be a while before they can be in .api they should be safe to use by other ETS libraries where it makes sense.

So for something like what you are planning for Envisage, it's probably worth experimenting with TraitList and if you run into trouble using it, then that's very good feedback to have which might inform further Traits development.

@kitchoi
Copy link
Contributor Author

kitchoi commented Jan 13, 2021

For reference, this is the Envisage PR which subclasses TraitList: enthought/envisage#354

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

No branches or pull requests

2 participants