Support standard tickdir control (in/out/inout) in axisartist.#30369
Support standard tickdir control (in/out/inout) in axisartist.#30369QuLogic merged 1 commit intomatplotlib:mainfrom
Conversation
a80878e to
6831cb5
Compare
|
Sure, done. |
timhoffm
left a comment
There was a problem hiding this comment.
Sorry, I'm a bit nit-picky today, but let's improve on code and docs, when we touch it anyway.
|
No worries, it's good to improve this... |
|
Some refs still need fixing: |
|
Hopefully the missing references are fixed. |
| # Remove this line when this test image is regenerated. | ||
| plt.rcParams['text.kerning_factor'] = 6 | ||
| plt.rcParams.update({ | ||
| "text.kerning_factor": 6, "xtick.direction": "in", "ytick.direction": "in"}) |
There was a problem hiding this comment.
Do you intend that all of these be removed in the text-overhaul branch, as text.kerning_factor will be?
There was a problem hiding this comment.
I pushed a second commit (as usual, without the actual baseline image updates) where I take advantage of the upcoming text-overhaul branch to update the tests as well; now they are a mix of "in", "out", and "inout" (to test a bit everything). Note that there are also other axisartist tests where there is a high tolerance currently set due to partial updates to the axisartist code; likely those could also have their tolerance set to zero again as the baselines will likely need to be regen'd anyways (I didn't touch them, but I can do it if you prefer -- it's just a grep for tol in the test suite of axisartist).
Let me know if you want me to retarget this to text-overhaul.
There was a problem hiding this comment.
I'd rather this get in through regular review, but maybe you can target the second commit to the text-overhaul branch? Not sure if that's too much work though.
There was a problem hiding this comment.
No problem at all, but the second commit only makes sense after the first (feature) goes in to main and main gets merged back into text-overview (so this PR is effectively blocking the final merge-back)? Unless we decide not to take advantage of the opportunity to update the baselines here to test the new functionality.
There was a problem hiding this comment.
Well, we already have one approval and I don't think much has changed since then, so I'm sure we can merge this one right now. I can merge main into text-overhaul after that.
There was a problem hiding this comment.
OK, so I dropped the testing commit from this PR; that commit exists now at anntzer@f1777c6 (aiot-tests branch). Right now that commit is atop the main branch (via this PR), I will rebase it atop text-overhaul once this PR is merged into main and main into text-overhaul, and open a PR for it.
(Note that the aiot-tests commit stills builds upon this PR because this one already alters the tests but in a way that fully maintains backcompatibility of baseline images so that tests locally pass; thus the aiot-tests commit also needs to undo these alterations.)
... and also respect rcParams, which makes axisartist ticks default to
pointing outwards, similarly to standard ticks.
Rework the tick direction machinery to also support "inout" (similarly
to standard ticks).
One may wonder whether external ticks should really be drawn in the
local direction of the gridlines, though, or whether drawing them
orthogonal to the axis spine would look better. This PR keeps the old
behavior of `set_ticks_out(True)`, but we could later add something like
``set_tick_direction("out_ortho")`` (name up to bikeshedding).
While at it, also deprecate the entirely unused
`Ticks.locs_angles_labels` and `LabelBase.locs_angles_labels`.
f1777c6 to
61b6cdc
Compare
... and also respect rcParams, which makes axisartist ticks default to pointing outwards, similarly to standard ticks.
Rework the tick direction machinery to also support "inout" (similarly to standard ticks).
One may wonder whether external ticks should really be drawn in the local direction of the gridlines, though, or whether drawing them orthogonal to the axis spine would look better. This PR keeps the old behavior of
set_ticks_out(True), but we could later add something likeset_tick_direction("out_ortho")(name up to bikeshedding).While at it, also deprecate the entirely unused
Ticks.locs_angles_labelsandLabelBase.locs_angles_labels.See the modified simple_axis_pad example:


and demo_curvelinear grid shows why "out_ortho" may actually be a better option:
PR summary
PR checklist