-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Draft: Contour kwarg for negative_linestyles #23102
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
Draft: Contour kwarg for negative_linestyles #23102
Conversation
lib/matplotlib/contour.py
Outdated
@@ -1764,6 +1769,17 @@ def _initialize_x_y(self, z): | |||
iterable is shorter than the number of contour levels | |||
it will be repeated as necessary. | |||
|
|||
negative_linestyles : {*None*, 'solid', 'dashed', 'dashdot', 'dotted'}, optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the proper way to break this line to meet line length requirements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*None* or string, optional
and then list the acceptable strings below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced it should be *None*
, as *...*
is used to mark parameter names, not values. Although it is clearly used in contour.py and in a few other locations.
Grepping a bit, a plain None
seems to be the most common option. (Although *None*
is used in > 100 locations in the code.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the same thing and found mixed results. I am happy to remove ... if that is the consensus.
I also found a mix of "None or str", and "str or None"...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hoping that someone will come in with a consensus statement. :-)
But I'm pretty sure that not using *None*
will be less controversial than using it. Also, I believe that in the text it should either be a plain None or a double single-backtick one (to render as code), although the former is clearly the most common.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in rst it's None, but not sure about the docstrings.
fa2eb31
to
bf46fec
Compare
lib/matplotlib/contour.py
Outdated
negative_linestyles=None, hatches=(None,), alpha=None, | ||
origin=None, extent=None, cmap=None, colors=None, norm=None, | ||
vmin=None, vmax=None, extend='neither', antialiased=None, | ||
nchunk=0, locator=None, transform=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd position the new kw-arg last.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for letting me know!
lib/matplotlib/contour.py
Outdated
neg_ls = mpl.rcParams['contour.negative_linestyle'] | ||
# If negative_linestyles was not defined as a kwarg, | ||
# define negative_linestyles with rcParams | ||
if not neg_ls: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not neg_ls: | |
if neg_ls is None: |
negative_linestyle=''
would otherwise lead to surprising results.
Should you also set self.negative_linestyles
here? I think it can make sense to not use the neg_ls
local variable and just use self.negative_linestyles
throughout the code here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, I'll move the case with rcParams['contour.negative_linestyle']
to where self.negative_linestyles
is initially defined.
bf46fec
to
20d7d81
Compare
Bumps [docker/setup-qemu-action](https://github.com/docker/setup-qemu-action) from 1 to 2. - [Release notes](https://github.com/docker/setup-qemu-action/releases) - [Commits](docker/setup-qemu-action@v1...v2) --- updated-dependencies: - dependency-name: docker/setup-qemu-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
read_text/write_text default to the locale encoding, which may or may not be utf8. Fix these by making the encoding explicit or by using bytes.
I don't think other encodings really worked previously, so there's no deprecation period.
For `FontProperties(stretch=12345)`; before: ``` Traceback (most recent call last): File ".../matplotlib/font_manager.py", line 888, in set_stretch raise ValueError() ValueError The above exception was the direct cause of the following exception: Traceback (most recent call last): File "<string>", line 1, in <module> File ".../matplotlib/font_manager.py", line 695, in __init__ self.set_stretch(stretch) File ".../matplotlib/font_manager.py", line 891, in set_stretch raise ValueError("stretch is invalid") from err ValueError: stretch is invalid ``` after ``` Traceback (most recent call last): File "<string>", line 1, in <module> File ".../matplotlib/font_manager.py", line 695, in __init__ self.set_stretch(stretch) File ".../matplotlib/font_manager.py", line 900, in set_stretch raise ValueError(f"{stretch=} is invalid") ValueError: stretch=12345 is invalid ```
For tex failures (e.g., `figtext(.5, .5, "\N{snowman}", usetex=True)`), instead of ``` Traceback (most recent call last): File ".../matplotlib/texmanager.py", line 253, in _run_checked_subprocess report = subprocess.check_output( File "/usr/lib/python3.10/subprocess.py", line 420, in check_output return run(*popenargs, stdout=PIPE, timeout=timeout, check=True, File "/usr/lib/python3.10/subprocess.py", line 524, in run raise CalledProcessError(retcode, process.args, subprocess.CalledProcessError: Command '['latex', '-interaction=nonstopmode', '--halt-on-error', '../71cab2b5aca12ed5cad4a481b3b00e42.tex']' returned non-zero exit status 1. The above exception was the direct cause of the following exception: Traceback (most recent call last): <a long traceback> raise RuntimeError( RuntimeError: latex was not able to process the following string: b'\\u2603' Here is the full report generated by latex: This is pdfTeX, Version 3.141592653-2.6-1.40.23 (TeX Live 2021) (preloaded format=latex) <the tex output> ``` instead report the failure in a single block, as ``` Traceback (most recent call last): <a long traceback> raise RuntimeError( RuntimeError: latex was not able to process the following string: b'\\u2603' Here is the full command invocation and its output: latex -interaction=nonstopmode --halt-on-error ../71cab2b5aca12ed5cad4a481b3b00e42.tex This is pdfTeX, Version 3.141592653-2.6-1.40.23 (TeX Live 2021) (preloaded format=latex) <the tex output> ``` (the exception chaining is not particularly informative, and we can move the only relevant info -- the command we tried to run -- to the second exception).
This fixes the following error in mingw gcc toolchain. Also clang also have same error. src/_tkagg.cpp:274:26: note: crosses initialization of 'bool tk_ok' 274 | bool tcl_ok = false, tk_ok = false; | ^~~~~ src/_tkagg.cpp:274:10: note: crosses initialization of 'bool tcl_ok' 274 | bool tcl_ok = false, tk_ok = false; | ^~~~~~ According to C++ standard (6.7.3): It is possible to transfer into a block, but not in a way that bypasses declarations with initialization. A program that jumps from a point where a variable with automatic storage duration is not in scope to a point where it is in scope is ill-formed unless the variable has scalar type...
This adds the rcParams savefig.directory option into the macosx backend for the savefig dialog window.
fill_between isn't the right thing to use here. It might do the same thing (as long as the plot isn't interacted with / zoomed out), but it feels real clunky and should not be encouraged to use here in a user facing example. I only encountered this because this plot was showing up as a usage example on the plt.fill_between() API docs (older version) and I found this real odd.
Arrays sometimes don't have all the methods arrays should have, so add another check here. Plot requires both ndim and shape and this will extract the numpy array if x does not have those attributes. Otherwise leave the object alone, because unit support (currently only in plot) requires the object to retain the unit info.
When an overunder symbol has a subscript, it goes below the symbol, and the baseline of the Vlist thus created is actually the baseline of the lowest item in the Vlist. Hence, it must be moved only if there is a subscript, and not otherwise.
Adds compress=True to compressed layout engine. Works for compact axes grids.
Closes matplotlib#23114, where somebody has installed matplotlib into another git repo.
The *math* parameter is passed through many layers of the call stack but is ultimately only used for a single purpose: deciding whether to replace the ASCII hyphen by a (longer) unicode minus. Instead of doing that, just do the substitution at the parsing stage. In particular, this fixes problematic unicode minus support with the "cm" fontset. This patch also reverts a significant part of 52003e4, as LogFormatters no longer need to pass unicode minuses in mathtext -- everything gets converted by mathtext. Likewise, this change also invalidates the test_log_scales baseline image (old, buggy wrt. unicode minus); replace it by a test that the drawn ticks are as expected (which was the intent in 90c1aa3).
AFAICT, (incorrect) support for the double-backslashed version was accidentally introduced in 027dd2c.
This also ensures consistency in the names in the error strings; see e.g. changes in test_mathtext (the contents of `\overline` are indeed a "body", not a "value"). Renaming "simple_group" to "optional_group" is semantically more meaningful.
082f63b
to
9e67dae
Compare
Moving to new branch: #23266 |
PR Summary
I made the
negative_linestyles
for contours toggle-able via kwarg. The only way to toggle this currently is withrcParams
.In the attached issue, we had some discussion on whether this is a good idea or not, but I think it is worth considering. We can maintain current functionality and just apply the kwarg if it is defined. This will not break previous functionality. I will make tests and write documentation/examples as well.
Please use this PR and the attached issue to discuss whether this is a good idea or not. I'm happy to take this in a different direction if there are some better ideas!
Closes #23028
Previous
With this PR
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).