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.
We noticed that
import param
wasn't enough to get the IPython display hook set up automatically while it should be given the code, instead the reactive user guide had to runimport param.ipython
for the hook to be set up.There were two bugs, the first one being that error raised when trying to import the
ipython.py
module fromparameterized.py
(cannot import name 'depends' from partially initialized module 'param.depends' (most likely due to a circular import) (/Users/mliquet/dev/param/param/depends.py)
) and the second one being catching the error with a baretry/except
.To fix this I had to move
register_display_accessor
to another module, that I calleddisplay.py
but maybe there's a better name likehooks.py
? I also inlined the imports required by theIPythonDisplay
class.To test this I had to refactor a little
parameterized.py
so it relies on a helper to find out whether we're withing an IPython context. My intention was to add a test to mock the IPython context, I managed to write a test that worked on its own but didn't manage in a reasonable amount of time to get it to work with the other tests. Posting that test here in case it's useful in the future:Even if I have submitted this fix, I'm not a big fan of having the IPython display hook automatically set up on
import param
and would rather have Param users explicitly call a function to register it.