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

✨ Add configuration for plot functions #288

Merged
merged 58 commits into from
Aug 25, 2024
Merged

Conversation

s-weigand
Copy link
Member

@s-weigand s-weigand commented Aug 25, 2024

Here is the long overdue PR that adds the plot config.
The main goal is to reduce repetitive boilerplate code (I.e. when changing plot labels).

See the difference in code to get the same resulting plot (CONFIG._reset(project_config) is just needed in the docs to show the differences.)
image

With the corresponding config:

plotting:
  general:
    default_args_override:
      linlog: true
      use_svd_number: true
      linthresh: 2
    axis_label_override:
      time: "Time (ps)"
      spectral: "Wavelength (nm)"
      data_left_singular_vectors: ""
      data_singular_values: "Singular Value (a.u.)"
      data_right_singular_vectors: ""

Change summary

  • ✨ Add configuration for plot functions
  • ⬆️🧰 Update tooling
  • 🧰📚 Change notebook doc parser to myst-nb (this allows support for mermaid)

Checklist

  • ✔️ Passing the tests (mandatory for all PR's)

s-weigand and others added 30 commits August 25, 2024 20:46
✨ Add PlotLabelOverRideValue and PlotLabelOverRideMap
# Conflicts:
#	requirements_pinned.txt
🩹 Fix mutation of  input values in plot config
🚇👌 Use non editable install in CI (closer to user how users run it)
This prevents the folloing warning when running the tests:

RuntimeWarning: More than 20 figures have been opened. Figures created through the pyplot interface (`matplotlib.pyplot.figure`) are retained until explicitly closed and may consume too much memory. (To control this warning, see the rcParam `figure.max_open_warning`). Consider using `matplotlib.pyplot.close()`.
👌 Improve error collection and messages
👌 Make Config.reload lazy (only reloaded when files changed)
👌 Make Config.reload respect plot_config_context
This allows users to pass a tuple of kwarg names (exclude_from_config) that should be ignored when generating the json schema
Changed the spelling of the class `PlotLabelOverRideValue` to the grammatically correct `PlotLabelOverrideValue`.
The term "override" is a single word, so it does not make sense to use "OverRide" in the class name.
Correcting `PlotLabelOverRideMap` -> `PlotLabelOverrideMap`
It should be Overdue not OverDue  (in OverDueDeprecationError). The word "overdue" is a single word and should not be split into "Over" and "Due" in the class name.
@s-weigand s-weigand requested a review from a team as a code owner August 25, 2024 19:00
Copy link
Contributor

sourcery-ai bot commented Aug 25, 2024

🧙 Sourcery has finished reviewing your pull request!


Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

Binder 👈 Launch a binder notebook on branch s-weigand/pyglotaran-extras/plot-config

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've reviewed this pull request using the Sourcery rules engine. If you would also like our AI-powered code review then let us know.

pyglotaran_extras/config/config.py Dismissed Show dismissed Hide dismissed
pyglotaran_extras/config/config.py Dismissed Show dismissed Hide dismissed
pyglotaran_extras/config/plot_config.py Fixed Show fixed Hide fixed
tests/config/test_config.py Dismissed Show dismissed Hide dismissed
Copy link

codecov bot commented Aug 25, 2024

Codecov Report

Attention: Patch coverage is 95.25862% with 22 lines in your changes missing coverage. Please review.

Project coverage is 58.26%. Comparing base (b880df9) to head (3b6133c).
Report is 27 commits behind head on main.

Files with missing lines Patch % Lines
pyglotaran_extras/config/config.py 94.33% 6 Missing and 3 partials ⚠️
pyglotaran_extras/config/plot_config.py 95.87% 4 Missing and 5 partials ⚠️
pyglotaran_extras/plotting/plot_svd.py 80.00% 2 Missing ⚠️
tests/io/test_load_data.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #288       +/-   ##
===========================================
+ Coverage   44.77%   58.26%   +13.48%     
===========================================
  Files          27       31        +4     
  Lines        1043     1598      +555     
  Branches      162      290      +128     
===========================================
+ Hits          467      931      +464     
- Misses        568      651       +83     
- Partials        8       16        +8     

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


🚨 Try these New Features:

Main reason to pin pydantic==2.7.2 at runtime is that this is the minimum requirement of pyglotaran 0.7.3

The conflict is caused by:
    The user requested pydantic==2.7
    pyglotaran 0.7.3 depends on pydantic>=2.7.2
As discussed before this allows better integratin and consistency with the config.

Since plot_pfid wasn't part of a release this isn't a regression.
@s-weigand
Copy link
Member Author

After this PR got merged our docs will have the config schema for the builtin functions under <root_url>/_static/pygta_config.schema.json and we can use it to add it to the schema store.
See for PR built of docs

This technically isn't needed since it can fallback using __getitem__ and use False on KeyError, however this causes the debugger to stop on __getitem__  unneededly.

Wenn to use the type object to make mypy happy, since the super type Mapping has that typing.
@jsnel
Copy link
Member

jsnel commented Aug 25, 2024

As we say in dutch "Het heeft even geduurd, maar nu hebben we ook wat!". 👍
Loosely translated: "It (sure) took a while, but it was (well) worth it!"

To illlustrate what can be done with it, here the plot_overview from the fluorescence case study without config and with.

image

# yaml-language-server: $schema=pygta_config.schema.json

plotting:
  general:
    default_args_override:
      linlog: false
      use_svd_number: false
      linthresh: 2
    axis_label_override:
      time: Time (ps)
      spectral: Wavelength (nm)
      data_left_singular_vectors: ''
      data_singular_values: Singular Value (a.u.)
      data_right_singular_vectors: ''
  plot_data_overview:
    default_args_override: 
      use_svd_number: false
  plot_svd:
    default_args_override:
      use_svd_number: true
  plot_overview:
    default_args_override:
      show_data: false

image

... which may not seem, until you realize all axis labels have now almost effortlessly been replaced (spectral -> Wavelength (nm), time -> Time (ps), etc).

The autocompletion, due to the (autogenerated) pygta_config.schema.json being present (which is created after calling CONFIG.init_project() works very well in VS Code.

from pyglotaran_extras import CONFIG

CONFIG.init_project()

image

All in all, an impressive piece of work, and clearly a labor of love, which I'm sure out community will ❤️ as well.

Copy link
Member

@jsnel jsnel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and tested ok. The docs can still do with some polishing here and there, but this is more than good enough for the pyglotaran_extras CONFIG v1 release. 👍

For the effort and pushing to finish in conjunction with the pyglotaran 0.7.3 release, I award you a gold medal 🥇 !

@jsnel jsnel merged commit 337bfe3 into glotaran:main Aug 25, 2024
31 checks passed
@s-weigand s-weigand deleted the plot-config branch August 25, 2024 21:07
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.

2 participants