Skip to content

Update plot_modifications.py #5116

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+23 −5
Open

Update plot_modifications.py #5116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a84111
Update plot_modifications.py
tiapac Feb 11, 2025
8611736
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 11, 2025
c9add43
Update plot_modifications.py
tiapac Feb 11, 2025
ff9b352
test contour callback cmap, color kwargs
chrishavlin Feb 24, 2025
a321d64
add test_callbacks_pytestonly to nose ignore lists
chrishavlin Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions nose_ignores.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@
--ignore-file=test_sph_pixelization_pytestonly\.py
--ignore-file=test_time_series\.py
--ignore-file=test_cf_radial_pytest\.py
--ignore-file=test_callbacks_pytestonly\.py
1 change: 1 addition & 0 deletions tests/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,5 +230,6 @@ other_tests:
- "--ignore-file=test_offaxisprojection_pytestonly\\.py"
- "--ignore-file=test_sph_pixelization_pytestonly\\.py"
- "--ignore-file=test_cf_radial_pytest\\.py"
- "--ignore-file=test_callbacks_pytestonly\\.py"
cookbook:
- 'doc/source/cookbook/tests/test_cookbook.py'
11 changes: 6 additions & 5 deletions yt/visualization/plot_modifications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,11 +961,12 @@ def __init__(
self.factor = _validate_factor_tuple(factor)
self.clim = clim
self.take_log = take_log
self.plot_args = {
"colors": "black",
"linestyles": "solid",
**(plot_args or {}),
}

self.plot_args = {**(plot_args or {})}
if "colors" not in self.plot_args and "cmap" not in self.plot_args:
self.plot_args["colors"] = "black"
if "linestyles" not in self.plot_args:
self.plot_args["linestyles"] = "solid"
Comment on lines +965 to +969
Copy link
Member

Choose a reason for hiding this comment

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

how about this instead ?

Suggested change
self.plot_args = {**(plot_args or {})}
if "colors" not in self.plot_args and "cmap" not in self.plot_args:
self.plot_args["colors"] = "black"
if "linestyles" not in self.plot_args:
self.plot_args["linestyles"] = "solid"
self.plot_args = {
"linestyles": "solid",
**(plot_args or {})
}
if "cmap" not in plot_args:
self.plot_args.set_default("colors", "black")

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for coming back at it after a while, I'm going thorugh particularly busy weeks. The change works for me, looks cleaner with set_default.
Should we do the same for linestyles?

Suggested change
self.plot_args = {**(plot_args or {})}
if "colors" not in self.plot_args and "cmap" not in self.plot_args:
self.plot_args["colors"] = "black"
if "linestyles" not in self.plot_args:
self.plot_args["linestyles"] = "solid"
self.plot_args = {**(plot_args or {})
}
if "cmap" not in plot_args:
self.plot_args.set_default("colors", "black")
if "linestyles" not in plot_args:
self.plot_args.set_default("linestyles", "solid")

Copy link
Member

Choose a reason for hiding this comment

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

No. linestyles is already being overriden correctly, let's keep the more verbose style only for when it's really needed.

Copy link
Member

Choose a reason for hiding this comment

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

Note that the if "linestyles" not in plot_args condition is entirely redundant in your suggestion, too.

self.label = label
self.text_args = {
"colors": "white",
Expand Down
15 changes: 15 additions & 0 deletions yt/visualization/tests/test_callbacks_pytestonly.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import pytest

import yt
from yt.testing import fake_amr_ds


@pytest.mark.parametrize("plot_args", [{"cmap": "jet"}, {"colors": "red"}])
def test_contour_callback_kwargs(plot_args):
ds = fake_amr_ds()
slc = yt.SlicePlot(ds, "x", ("stream", "Density"))
slc.annotate_contour(("stream", "Density"), plot_args=plot_args)

pargs = slc._callbacks[0].plot_args
assert all(pargs[ky] == val for ky, val in plot_args.items())
_ = slc.render()
Loading