-
Notifications
You must be signed in to change notification settings - Fork 532
ENH: Allow chained name_source #938
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
we should definitely add tests for this. |
ns = trait_spec.name_source | ||
while isinstance(ns, list): | ||
if len(ns) > 1: | ||
iflogger.warn('Only one name_source per trait is allowed') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is only one allowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, we have a somewhat difficult decision here.
Former implementation iterated the name_source list until it found the first defined name_source.
Current implementation may accept undefined name_sources, but they must have a name_source as well.
In both implementations only one name_source is actually used, right?.
Hi @satra, I think this PR is ready to be considered. I've updated the description at the top, and the WIP flag replaced. |
Conflicts: CHANGES
An input trait with
name_source
defined and pointing to another trait that definesname_source
pointing to a third trait now is sequentially resolved, adding the appropriate suffixes.