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

Fix rticks(), rtickformat(), rticklabels() and rtickangle() being ignored #356

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

j0hnnybash
Copy link
Contributor

The rtick* group of functions previously had no effect unless the the r-axis was manually made visible. The r-axis is invisible by default and instead the major grid is enabled by default (which is much more helpful for polar plots), however this had the undesirable fact that code block that handles the *tick*() functions for arbitrary axis would not be executed.

Another related issue which is also fixed by this change is that both ax->rticklabels({}); and ax->rticks({}); would not hide the tick labels, since hiding tick labels without also hiding the grid requires special handling.

@alandefreitas
Copy link
Owner

alandefreitas commented Jun 17, 2023

undesirable fact that code block that handles the tick() functions for arbitrary axis would not be executed

I'm not sure I understand this sentence. Would you have an example?

Obs: would you mind using conventional commits and present tense? That allows commits to be properly incorporated into the changelog. Ex: fix: r-ticks are not ignored when ...

In other projects I did include https://github.com/alandefreitas/futures/blob/7442a3cb0e20ce86f280f24e83f1b0bc478b2d04/.github/workflows/build.yml#L131-L133 but matplot++ doesn't have that.

@j0hnnybash
Copy link
Contributor Author

I'm not sure I understand this sentence. Would you have an example?

Reading my own comment back, I realize it's neither understandable not grammatically correct...

I'll try to come up with some examples (also for the documentation) based on my test code.

@alandefreitas
Copy link
Owner

Oh... you included complete examples. :) I meant just small inline examples of what you mean in the PR. The new examples are fine, but they require new images for the docs, etc... But I think I get the point now.

@alandefreitas
Copy link
Owner

Matplotplusplus / MacOSX/10.15/Static/X64/Release (pull_request) Failing after 5m

Uh-oh

@j0hnnybash
Copy link
Contributor Author

Maybe I should dumb down the example...
The OSX build seems to have trouble finding std::cyl_bessel_i, but this should be available in <cmath> with C++17.

Okay, apparently libc++ does not support all C++17 library features yet, "Mathematical Special Function" specifically (see), so I'll have to find a simpler and perhaps also more visually appealing example.

@alandefreitas
Copy link
Owner

The OSX build seems to have trouble finding std::cyl_bessel_i, but this should be available in with C++17.

Yes. It's common for some STL features never to get really implemented depending on the demand for them. The C++ garbage collector got approved and deprecated without ever being really implemented by anyone.

Maybe I should dumb down the example...

You can remove the examples if you want. We just needed them for the discussion here. If we're keeping these examples, we need to add them to README.md so they can be picked up by the documentation generator.

@j0hnnybash
Copy link
Contributor Author

Sorry for taking so long.

I've removed the examples, hopefully I'll get around to properly integrate some of them some other time.

@alandefreitas
Copy link
Owner

Could you please rebase this PR on top of master or develop? The PR is now behind these branches because there has been a release.

Context: For polar plots the (unsightly, and unhelpful) r-axis is
hidden by default, instead the grid is enabled by default.

Thus handle rticks(), rtickformat(), rticklabels() and rtickangle()
and generate the corresponding gnuplot commands even when the r-axis
is hidden.

Additionally, handle `rticks({})` and `rticklabels({})` properly by
essentially setting `rtickformat("")`, this ensures that labels are
hidden without having to disable the grid.
@j0hnnybash
Copy link
Contributor Author

FYI: I rebased onto master.

@alandefreitas alandefreitas merged commit 7af365c into alandefreitas:master Jul 17, 2023
@alandefreitas
Copy link
Owner

FYI: I rebased onto master.

Great! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants