allow setting HistoryManager.hist_file with config #940
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fixes improper init in HistoryAccessor, preventing setting `hist_file` via config from having any effect. Also remove dependency on having a shell object in `HistoryAccessor.__init__` Associated test included.
|
|
||
| def test_hist_file_config(): | ||
| cfg = Config() | ||
| cfg.HistoryManager.hist_file = tempfile.mktemp() |
There was a problem hiding this comment.
mktemp is deprecated. For this particular test, I suggest instead creating the temp file with NamedTemporaryFile:
tfile = tempfile.NamedTemporaryFile(delete=False)
cfg.HistoryManager.hist_file = tfile.name
...
try:
nt.assert...
finally:
os.unlink(tfile.name)That's the right pattern for these guys. In Pyhton 3 they've been turned into context managers, which is really what you want.
|
Other than the above fixes to the test, the code looks good to me. I'll merge it once this small stuff is fixed; thanks! |
|
suggested changes made and pushed |
fperez
added a commit
that referenced
this pull request
Oct 30, 2011
Allow setting HistoryManager.hist_file with config. This fixes trait init in HistoryAccessor, which prevented setting hist_file via config from having any effect.
mattvonrocketstein
pushed a commit
to mattvonrocketstein/ipython
that referenced
this pull request
Nov 3, 2014
Allow setting HistoryManager.hist_file with config. This fixes trait init in HistoryAccessor, which prevented setting hist_file via config from having any effect.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes trait init in HistoryAccessor, which prevented setting
hist_filevia config from having any effect.Also remove dependency on having a shell object in
HistoryAccessor.__init__Associated test included.
as described in #882