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

Add tests and docs for xgetattr and xsetattr #1321

Open
mdickinson opened this issue Oct 23, 2020 · 1 comment
Open

Add tests and docs for xgetattr and xsetattr #1321

mdickinson opened this issue Oct 23, 2020 · 1 comment

Comments

@mdickinson
Copy link
Member

From #1239 (comment):

xgetattr should really get tests, a decent docstring, and arguably fixes, too: the current version does different things for intermediate lookups than for the final lookup, for no obvious reason: for intermediate lookups, an actual value of None is replaced with default. For the final lookup, a value of None is left as None.

@mdickinson
Copy link
Member Author

On the fixes front, it's hard to change behaviour here since external packages are already using xgetattr or xsetattr.

This may be a good opportunity to think again about the behaviour we actually want, particularly with respect to None-handling, missing attributes, and default values, and recodify it under a better name or in a better place.

If the intent is to support retrieval of values represented by extended trait names on a HasTraits class, one option may be a method on HasTraits. (That would preclude re-using the same logic for doing extended imports, but that's a different use-case, and seems like a misuse of xgetattr in the first place.)

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

1 participant