-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Dynamic inheritance in OptionTrees #796
Conversation
Thanks for that last fix! Hope the tests pass this time... |
The existing tests passed and I am currently adding more unit tests. I should also mention that there is no need to change |
I looked into the possibility of defining a utility to replace this code for generating a string specification: components = (obj.__class__.__name__,
group_sanitizer(obj.group),
label_sanitizer(obj.label))
'.'.join([c for c in components if c]) But decided this pattern doesn't appear often enough in the codebase to bother with after grepping for Having added unit tests to ensure the new dynamic inheritance keeps working, I think this PR is now ready for a review/merge. |
I added one more unit test to simulate the %%opts magic. Ready to review/merge. |
I just updated the title of the PR as I confused myself thinking it is about options on |
Thanks for adding the tests. Very happy we finally fixed this. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This PR is primarily to find out whether disabling
merge_keywords
has an effect on the test. If the tests continue to pass, then I propose keeping it disabled.Edit: This PR turned into a fix for dynamic inheritance.