Skip to content

Add unit axis attribute, for Plots v1 #5098

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Ickaser
Copy link
Contributor

@Ickaser Ickaser commented Jun 6, 2025

Description

Essentially identical to #5095 , but made against the master branch so that I'm not waiting for Plots v2--folder structures are different enough that it made sense to open as a separate PR.

@gustaphe and I discussed some in #5095; it would be helpful to get feedback from @t-bltg or @BeastyBlacksmith or someone else who is not as focused on Unitful usage.

A lot of the noise in the diff is a result of switching to Plots.get_guide(axis) instead of axis[:guide], because that new function allows us to more elegantly handle differing unit formats in the guide strings; in any case where the axis has no units, the result of get_guide(axis) is axis[:guide] so not a big change in behavior.

Other changes involve moving some of the machinery out of the UnitfulExt so that it is accessible within Plots for axis handling.

Attribution

Things to consider

  • Does it work on log scales?
  • Does it work in layouts?
  • Does it work in recipes?
  • Does it work with multiple series in one call?
  • PR includes or updates tests?
  • PR includes or updates documentation?

@t-bltg
Copy link
Member

t-bltg commented Jun 7, 2025

CI is failing on this one.

Copy link

codecov bot commented Jun 9, 2025

Codecov Report

Attention: Patch coverage is 98.14815% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.80%. Comparing base (fa65e7d) to head (2e0f267).
Report is 45 commits behind head on master.

Files with missing lines Patch % Lines
src/axes.jl 96.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5098      +/-   ##
==========================================
+ Coverage   89.77%   89.80%   +0.03%     
==========================================
  Files          40       40              
  Lines        8780     8867      +87     
==========================================
+ Hits         7882     7963      +81     
- Misses        898      904       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Ickaser
Copy link
Contributor Author

Ickaser commented Jun 9, 2025

This PR isn't quite ready yet, because I think it deserves a few more tests for the UnitfulExt before merging, plus some additions to the docs. But @t-bltg the CI failures seem to be either 1) a resurface of a PlotlyJS>Blink>Electron issue #5075, which is randomly hitting only some of the Ubuntu test cases or 2) something to do with the 1.12 prerelease

@t-bltg
Copy link
Member

t-bltg commented Jun 9, 2025

  1. a resurface of a PlotlyJS>Blink>Electron issue fix PlotlyJS ECONNREFUSED #5075

#5100

  1. something to do with the 1.12 prerelease

Ignore those (they are development aids), we focus on julia release and lts.

@t-bltg t-bltg added the enhancement improving existing functionality label Jun 9, 2025
@Ickaser Ickaser mentioned this pull request Jun 13, 2025
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improving existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants