Fix image metadata being used in wrong cell#599
Fix image metadata being used in wrong cell#599flying-sheep wants to merge 2 commits intoexecutablebooks:masterfrom
Conversation
There was a problem hiding this comment.
I changed the metadata of the plot to reflect the text/plain representation (<Figure size 432x288 with 1 Axes>). This results in the plot looking blurry since the image data’s resolution is smaller (390×248). While it doesn’t look nice in the test notebook, it makes the most sense like that.
(The perfect solution would be to have quadruple-sized “retina” image data – e.g. 800×600 – with the metadata being half that – 400×300 – which would match real-world usage)
OriolAbril
left a comment
There was a problem hiding this comment.
Changes look good and I didn't notice anything strange after testing the PR locally. Thanks for the quick fix @flying-sheep!
|
Hi! It would be great to have this fix into the next release 😄. |
|
Thanks for your feedback everyone! I am doing a round of maintenance on the Sphinx stack at the moment, I will aim for a release next week. |
|
Hi @agoose77 are you still planning to do a release soon? |
|
Thanks @flying-sheep, this is very helpful. Another contributor also opened a PR #609, so I'm in the fortunate position of having to make a choice between the two! As there's a new test in #609, I'm going to close this in favour of that one :) |
|
@agoose77 I would merge both PRs actually. They do have the addition of the If you prefer, I can also merge both PRs into one keeping the commits of each author so contributions are properly tracked and open a new with the changes from both PRs |
|
@OriolAbril yep, I'm pulling the changes from the tests to attribute @flying-sheep :) |
Fix bug pointed out in #588 (review) caused by reusing the image_options dict. Fixes #605.
Seems like in my previous PR I didn’t understand the tests enough to realize that I changed them to nonsense, but now I do.
Sorry for that!
cc @OriolAbril