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

Improve ruler #4005

Merged
merged 9 commits into from
Dec 21, 2023
Merged

Improve ruler #4005

merged 9 commits into from
Dec 21, 2023

Conversation

vallsv
Copy link
Contributor

@vallsv vallsv commented Dec 14, 2023

This PR improve the ruler from the plot with a default fixed dashed style

image

No need changelog, it's a new feature

  • Move PlotToolButton as an independent module
  • Move RulerToolButton as an independent module
  • Move inner class as a private class
  • Update the style

@vallsv vallsv requested review from t20100 and payno December 14, 2023 19:32
@t20100 t20100 added this to the 2.0.0 milestone Dec 15, 2023
@payno
Copy link
Member

payno commented Dec 15, 2023

short answer: fine with me 👍 . Try to get something as much as possible 'readable' by default for colormap like 'grayscale' and that's it.

@t20100
Copy link
Member

t20100 commented Dec 15, 2023

Regarding dashed line support, as a general rule, I prefer to stick to matplotlib naming/default style whenever possible and to have the OpenGL backend behave as close as possible to it.

So I'd prefer to add support for (offset, (dashes)) as in matplotlib set_linestyle than defining our own names.
I'll check how complex this is (it shouldn't be) and sync OpenGL dashes with default from matplotlib.
Anyway, this is not for this PR.

@t20100
Copy link
Member

t20100 commented Dec 15, 2023

BTW, did you try with dashed dot: -.?

@vallsv
Copy link
Contributor Author

vallsv commented Dec 15, 2023

BTW, did you try with dashed dot: -.?

I just try, that's not very nice

@vallsv
Copy link
Contributor Author

vallsv commented Dec 15, 2023

Regarding dashed line support, as a general rule, I prefer to stick to matplotlib naming/default style whenever possible and to have the OpenGL backend behave as close as possible to it.

So I'd prefer to add support for (offset, (dashes)) as in matplotlib set_linestyle than defining our own names. I'll check how complex this is (it shouldn't be) and sync OpenGL dashes with default from matplotlib. Anyway, this is not for this PR.

Only supporting the subset (offset, (dash1, dash2)) would be already very nice, and i will try to use it for sure.

@t20100
Copy link
Member

t20100 commented Dec 15, 2023

ok, then I'll add this

@vallsv
Copy link
Contributor Author

vallsv commented Dec 20, 2023

Everything was rebased using the new features from the main branch.

Still some commit could be cherry picked alone to the main branch, but it's mostly typing.

This can be reviewed.

@vallsv vallsv changed the title Draft: Improve ruler Improve ruler Dec 20, 2023
@payno
Copy link
Member

payno commented Dec 20, 2023

Fine with the source code, works well on my side. I don't have any strong opinion regarding splitting out the PlotToolButton.

@t20100 t20100 merged commit f9efde9 into silx-kit:main Dec 21, 2023
2 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants