Skip to content
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

Add extra.utils.imshow2 for easier image plotting #38

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Conversation

JamesWrigley
Copy link
Member

I wrote this function ages ago and I'm constantly copying it over to new notebooks, so I figured it might be useful to put here.

@JamesWrigley JamesWrigley added the enhancement New feature or request label Aug 4, 2023
@JamesWrigley JamesWrigley self-assigned this Aug 4, 2023
Comment on lines 58 to 63
# Try to avoid repeatedly importing matplotlib by looking in sys.modules
# first.
if "matplotlib.pyplot" in sys.modules:
plt = sys.modules["matplotlib.pyplot"]
else:
import matplotlib.pyplot as plt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Try to avoid repeatedly importing matplotlib by looking in sys.modules
# first.
if "matplotlib.pyplot" in sys.modules:
plt = sys.modules["matplotlib.pyplot"]
else:
import matplotlib.pyplot as plt
import matplotlib.pyplot as plt

Importing something a second time is cheap - it's basically just doing the same lookup in sys.modules that you do here. So there's no need to complicate it. 🙂

In [1]: %time import matplotlib
CPU times: user 257 ms, sys: 235 ms, total: 492 ms
Wall time: 445 ms

In [2]: %time import matplotlib
CPU times: user 21 µs, sys: 0 ns, total: 21 µs
Wall time: 32.2 µs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah cool, I thought it would re-import the whole thing. Fixed in 7906c54.


ax = plt

imshow_kwargs = dict(interpolation="none")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the docstring mention this too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, yes 🙈 Added in 7906c54.

Copy link
Collaborator

@philsmt philsmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, this looks like it may save some time regularly. Not a fan of the name, but I also don't got any other ideas.

Nothing to complain, just an idea: I regularly use norm=LogNorm() and always forget to add from matplotlib.colors import LogNorm. Maybe extend the interface by some lognorm=True option?

Still LGTM.

@JamesWrigley
Copy link
Member Author

Nothing to complain, just an idea: I regularly use norm=LogNorm() and always forget to add from matplotlib.colors import LogNorm. Maybe extend the interface by some lognorm=True option?

Good idea, added in b66a3e8 👍

@JamesWrigley JamesWrigley merged commit 2bcb83e into master Aug 4, 2023
4 checks passed
@JamesWrigley JamesWrigley deleted the imshow2 branch August 4, 2023 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants