-
-
Notifications
You must be signed in to change notification settings - Fork 201
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
Backport PR 522 to 4.3.x - Expose loaded config files, make load idempotent #532
Conversation
This change only takes a very small port of PR 341. Specifically, its a small portion of commit eaf1ae8 that introduces the List `_loaded_config_files` and returns a tuple from `load_config_file()`. This was necessary since the rest of this commit and the other 5 commits introduced API-level changes that were dependent on other changes not pertinent to a minor release.
d737580 Expose loaded config files, make load idempotent 61f25a7 Use more compatible form of copy for lists f044185 make comment more clear
In the absence of any other feedback, I'm going to merge this and then do a 4.3.3.dev release tomorrow or later tonight. |
@rmorshea - Is there anything I can help with to get 4.3.3 done? Thanks. |
Sorry @kevin-bates I've been really busy. Promise I'll get this done today. |
@@ -583,21 +584,26 @@ def _load_config_files(cls, basefilename, path=None, log=None, raise_config_file | |||
" {1} has higher priority: {2}".format( | |||
filename, loader.full_filename, json.dumps(collisions, indent=2), | |||
)) | |||
yield config | |||
yield (config, loader.full_filename) |
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.
FWIW, this is a private API that was indeed used by IPython until 5.5, so this patch-release of traitlets will break IPython <5.5 (5.8 is the last supporting Python 2 and 7.8 is current). The API that called this was somewhat rarely used (mainly in IPython.embed()
).
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.
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.
Good information @minrk - thank you.
This PR consists of 4 commits which should probably not be squashed when merged.