Skip to content

Conversation

@bwalraven
Copy link
Contributor

@bwalraven bwalraven commented Oct 10, 2025

@codecov
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.88%. Comparing base (627dd69) to head (df72f67).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #109      +/-   ##
==========================================
+ Coverage   98.75%   98.88%   +0.13%     
==========================================
  Files           7        7              
  Lines         723      808      +85     
==========================================
+ Hits          714      799      +85     
  Misses          9        9              

☔ 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.

@bwalraven
Copy link
Contributor Author

It appears testing matplotlib outputs is what is throwing the last errors. The errors are not (only) produced by the new functions added, but by older ones already merged. Options to fix this across all test files using matplotlib at once (according to chatgpt) include:

  1. adding
env:
  MPLBACKEND: Agg

to workflow actions,
or
2. creating a conftest.py file containing

import matplotlib
matplotlib.use("Agg")

@cchwala
Copy link
Member

cchwala commented Oct 11, 2025

@bwalraven Thanks for this PR. I will have a brief look at the CI errors.

@cchwala
Copy link
Member

cchwala commented Oct 11, 2025

The problem indeed does not stem from this PR. I reran CI in #106 just to see how it works there.

Maybe the errors, which are only in the windows CI runs, stems from actions/runner-images#12677

@cchwala
Copy link
Member

cchwala commented Oct 11, 2025

It is strange that this error regarding tkinter only appears in test_plot_confusion_matrix_count() when fig, ax = plt.subplots() is called. In test_plot_map the functions that are called also do use plt.subplots(). I see that there _, ax = plt.subplots() is used when calling plot_plg(). That should not make a difference, though (I think...).

@cchwala
Copy link
Member

cchwala commented Oct 11, 2025

There is a test in test_plot_map that does

 fig, ax = plt.subplots(figsize=(9, 3))
    _ = plg.plot_map.scatter_lines(x0, y0, x1, y1, c="g", ax=ax)

and this does not fail.

So, why does the

fig, ax = plt.subplots()

in test_plot_confusion_matrix_count fail 🤷 with this tcl or tkinter error?

C:\hostedtoolcache\windows\Python\3.11.9\x64\Lib\tkinter\__init__.py:2345: TclError

@bwalraven
Copy link
Contributor Author

Good question.. the error also doesn't appear for the python3.10 version, which (coincidentally?) is what I ran locally when writing the functions.

that hopefully fixes the issues with windows CI runs
@cchwala
Copy link
Member

cchwala commented Oct 11, 2025

@bwalraven I have now set the CI config to use windows_2022 instead of windows_latest, which would point to windows_2025 since the Github update from actions/runner-images#12677.

Now all test pass 🎉 and test coverage looks great 👍.

I had a very very brief look at the code and the updated notebook. Things look fine to me and the PR could be merged now.

Is there anything where you think a more detailed review or discussion of your implementation is required? Since this PR is about plotting functions, the details of the implementation are IMO not that crucial.

@bwalraven
Copy link
Contributor Author

Thanks for your efforts on this @cchwala!
I also don't think a more detailed discussion is needed, especially not on the last part I added. These functions are just the remaining ones from RAINLINK that I use more often, but were not in poligrain yet. Since I had the python code lying around already I just thought it was about time to add it to poligrain. So if it's up to me, its ready to be merged too! 🎉

@cchwala cchwala merged commit 6ff2f7c into OpenSenseAction:main Oct 11, 2025
18 checks passed
@bwalraven bwalraven deleted the plot_metadata branch October 11, 2025 19:52
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.

Add remaining meta data plots from RAINLINK

2 participants