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 handling of multi-series series_annotations #4225

Merged

Conversation

nick4f42
Copy link
Contributor

@nick4f42 nick4f42 commented Jun 2, 2022

Currently, values for series_annotations can be:

  • Vector of text e.g. ["foo", "bar"]
  • (x, args...) where x is a vector of text and args are the font/marker options
  • 1-row matrix of vectors of text e.g. permutedims([["foo"], ["bar"]])

Changes were made for specifying annotations for multiple series:

  • Matrix of text (one column for each series)
  • 1-row matrix where each element is x or (x, args...) where x is a vector of text and args are the font/marker options
  • A tuple (x, args...) where x is one of the previous two
    • The args in the inner tuples override the args in the outer tuple

Fixes #4073

@codecov
Copy link

codecov bot commented Jun 2, 2022

Codecov Report

Merging #4225 (127d9a4) into master (39b1913) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 127d9a4 differs from pull request most recent head 2e1a4d2. Consider uploading reports for the commit 2e1a4d2 to get more accurate results

@@            Coverage Diff             @@
##           master    #4225      +/-   ##
==========================================
+ Coverage   77.84%   77.89%   +0.05%     
==========================================
  Files          28       28              
  Lines        7083     7095      +12     
==========================================
+ Hits         5514     5527      +13     
+ Misses       1569     1568       -1     
Impacted Files Coverage Δ
src/components.jl 87.81% <100.00%> (+0.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39b1913...2e1a4d2. Read the comment docs.

@BeastyBlacksmith
Copy link
Member

That looks great, thank you. Can we add one more test for the matrix case?

Nick OBrien added 3 commits June 4, 2022 11:16
The code now handles the following values of `series_annotations`:

- Matrix of `String`/`PlotText`/etc (one column for each series)
- 1-row matrix of `AbstractVector` or `Tuple{AbstractVector, ...}`
- A tuple of `(x, ...)` where `x` is one of the previous values
@nick4f42 nick4f42 force-pushed the fix-nd-series-annotation branch from 127d9a4 to 2e1a4d2 Compare June 4, 2022 16:31
@nick4f42
Copy link
Contributor Author

nick4f42 commented Jun 4, 2022

That looks great, thank you. Can we add one more test for the matrix case?

There was already one for the matrix case but I just changed it to use [a c; b d] instead of [[a, b] [c, d]] so it's more clear that it's a matrix of scalars and not a row matrix of vectors. I also added another test case for the tuple(text_matrix, ...) case.

Let me know if there's anything else that should be changed!

@BeastyBlacksmith
Copy link
Member

Great, thank you!

@BeastyBlacksmith BeastyBlacksmith merged commit 6938f8d into JuliaPlots:master Jun 7, 2022
@nick4f42 nick4f42 deleted the fix-nd-series-annotation branch June 7, 2022 23:22
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.

[BUG] series_annotation does not slice correctly
2 participants