Adapt pysteps to allow for postprocessing plugins #405

joeycasey87 · 2024-07-19T14:10:32Z

Added an infrastructure.py file to the postprocessing folder which should operate in the same way as the interface file does for the importers in the io folder. The postprocessor file is currently effectively empty as no postprocessor plugins have been added yet. A line has been added to the init file so that it should act similar to the init file in the io folder.

dnerini · 2024-07-22T06:19:58Z

pysteps/postprocessing/__init__.py

@@ -2,3 +2,4 @@
 """Methods for post-processing of forecasts."""

 from . import ensemblestats
+from postprocessors import *


Suggested change

from postprocessors import *

from .postprocessors import *

dnerini · 2024-07-22T06:25:53Z

pysteps/postprocessing/postprocessors.py

just wondering if "postprocessors" is a good name? sounds a bit too vague, particularly as a submodule of "postprocessing" ... can we try to be a bit more specific? perhaps use "diagnostic" instead? what do you think? @ladc ?

Yes good point - "diagnostic" makes sense if we also want to add other postprocessors in the future which are not purely diagnostic (such as bias correction methods).

codecov · 2024-07-25T12:07:58Z

Codecov Report

Attention: Patch coverage is 93.38843% with 8 lines in your changes missing coverage. Please review.

Project coverage is 84.37%. Comparing base (9dc68c5) to head (657e56e).

Files with missing lines	Patch %	Lines
pysteps/postprocessing/interface.py	88.57%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #405      +/-   ##
==========================================
+ Coverage   84.23%   84.37%   +0.13%     
==========================================
  Files         160      161       +1     
  Lines       13242    13345     +103     
==========================================
+ Hits        11155    11260     +105     
+ Misses       2087     2085       -2

Flag	Coverage Δ
unit_tests	`84.37% <93.38%> (+0.13%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

joeycasey87 · 2024-08-13T13:51:24Z

Hi @dnerini, could you please review this pull request when you have the time? The extended version of the cookiecutter plugin template which can be used to create importer, diagnostic postprocessing, and nowcast plugins is nearly finished but I require the postprocessing interface to be merged to ensure that everything is behaving correctly when tested. Cheers

ladc · 2024-08-20T15:50:28Z

Thanks for this contribution! We discussed how to make it more generic and @joeycasey87 will try to refactor it tomorrow to allow for various kinds of post-processors including new ensemblestats and bias correction methods, for example. To be continued.

dnerini · 2024-09-08T18:56:52Z

what's the status of this PR? should we mark it as a draft?

Also reformatted with black. Tests were added for the get_method and diagnostics_info functions in the postprocessing interface. Tests for the discover_diagnostics function will be written once these changes have been merged as then the cookiecutter plugins can then be properly tested.

Changed the test file to match the test updated pysteps test file

These were necessary for the IO plugins, but less so for the diagnostics.

Avoid duplicate code, refactor into functions. Also fix a small typo causing a bug: postprocessor_s_

ladc · 2025-01-14T14:38:03Z

Apologies for the force-push, but I have done a rebase to the latest master version. The plan is now to clean up some of the dummy code (maybe move this into test, but most likely remove it entirely), add the necessary test(s) in test_plugins_support.py and hopefully merge it into the master branch soonish, thanks in advance to @FelixE91 #nopressure

For the tests, we're now only checking the importer cookiecutter template. The new precipitation type plugin was created using the diagnostics template (I suppose), so we should add a test for this new template ( see also https://github.com/pySTEPS/pysteps/blob/e3325854dae006b1e78eb43a6d2203f2b0b71560/pysteps/tests/test_plugins_support.py#L17C1-L17C20 )

Question: do we really need a separate template for all the plugin types? (Diagnostics, ensemblestats ... which are all in fact postprocessors)? Could we have one for any kind of postprocessors? Or even better, one cookiecutter template for all kinds of plugins which could even expose multiple functions (e.g. diagnostics and visualisation)? Not urgent at all.

…lugins

- should match names as in the plugin cookiecutter

- diagnostic plugins created with the cookiecutter are now correctly recognized and implemented

- importer and diagnostic plugins correctly recognized in entry points - cleaning: removed unused import modules

…o postprocessor_plugin

…nd diagnostic plugin.

ladc

Almost done, some final cleanup needed. There's also still some duplicate code but that will take a bit more time to simplify.

ladc · 2025-02-26T23:27:14Z

pysteps/postprocessing/diagnostics.py

Could we just delete this file?

I don't think so, at least not until we change the structure. The objective was to create a structure similar to what exists for the importers with io.importers.py and for the ensemblestats with postprocessing.ensemblestats.py.
The postprocessing interface.py and __init__.py currently read from diagnostics.py to check for available pre-installed functions¹ - that's consistent with the way pre-installed ensemblestats functions are loaded and similar to the io.__init__.py and io.interface.py to load pre-installed importers.

New functions found in the entry_points are added to the available methods in the methods dictionary.

The (io and postprocessing) interface also reads from the module (importers.py and ensemblestats.py|diagnostics.py, respectively) BUT it would not add the functions to the methods dictionary. Instead, they must be added explicitly to the interface.py methods dictionary (_importer_methods and _ensemblestats_methods|_diagnostics_methods, respectively) to avoid an error in the get_method() function.
This might be fixed to add pre-installed functions from the module automatically to the methods dictionary.

Footnotes

At the moment there are no pre-installed diagnostics. This might change in the future, unless we always use the cookiecutter to create plugins ↩

pysteps/postprocessing/ensemblestats.py

pysteps/postprocessing/interface.py

ladc · 2025-02-26T23:39:26Z

Also, by updating the cookiecutter template I just realised that we broke the standard pysteps tests, so best to merge these changes in quite fast.

FelixE91 · 2025-02-27T14:44:05Z

Additional tests are successful. CodeCov's getting closer to the target value. Still some work to do before we can merge into main.

FelixE91

Let's decide together whether we want to merge it like this or make some of the recommended changes first. @dnerini @ladc

FelixE91 · 2025-02-27T18:51:10Z

pysteps/postprocessing/interface.py

+from pprint import pprint
+import warnings
+
+_diagnostics_methods = dict()


There is still a discrepancy here, as the dictionary in the io.interface is called _importer_methods, i.e. without “s” on importer. However, I would leave it as it is, as the use of plural is consistent with the module name.

FelixE91 · 2025-02-27T18:53:40Z

pysteps/postprocessing/interface.py

+    difference = module_methods_set ^ interface_methods_set
+    if len(difference) > 0:
+        # print("\nIMPORTANT:")
+        _diff = module_methods_set - interface_methods_set


Since we want to avoid differences, it should not be a problem that these lines with if len(difference) > 0 are not considered in the tests - this means that the plugin implementation works well.

FelixE91 · 2025-02-27T18:55:00Z

ci/test_plugin_support.py

-
-import_abc_xxx("filename")
-import_abc_yyy("filename")
+from pysteps.io.importers import import_institution_name


Should similar lines be added to test the diagnostics plugin? Or not needed here in ci?

FelixE91 · 2025-02-27T18:57:35Z

pysteps/io/interface.py

@@ -107,7 +113,7 @@ def importers_info():

    difference = available_importers ^ importers_in_the_interface
    if len(difference) > 0:
-        print("\nIMPORTANT:")
+        # print("\nIMPORTANT:")


FelixE91 · 2025-02-27T18:59:55Z

pysteps/postprocessing/diagnostics.py

+"""
+
+# Add your diagnostic_ function here AND add this method to the _diagnostics_methods
+# dictionary in postprocessing.interface.py


Let's see how we proceed with this file. I think we must keep it as mention in Originally posted by @ladc in #405 (comment)

FelixE91 · 2025-02-27T19:04:59Z

pysteps/postprocessing/interface.py

+"""
+import importlib
+
+from pysteps.postprocessing import diagnostics, ensemblestats


Codacy Static Code Analysis does not like the import of diagnostics as it isn't explicitly called. However, it is needed to add diagnostics plugins as globals()[module] in add_postprocessor() needs the diagnostics module to be imported.

FelixE91 · 2025-02-27T19:09:42Z

tox.ini

@@ -65,9 +65,8 @@ commands =
    # Test the pysteps plugin support
    pip install cookiecutter
    cookiecutter -f --no-input https://github.com/pySTEPS/cookiecutter-pysteps-plugin -o {temp_dir}/
-    pip install {temp_dir}/pysteps-importer-abc
+    pip install {temp_dir}/pysteps-importer-institution-name


We must remember to update all these default names when we further develop or simplify the cookiecutter.

Maybe we can introduce an environment variable that sets the default name? Then we would only need to change it in one place.

FelixE91 · 2025-02-27T19:12:46Z

pysteps/postprocessing/interface.py

+
+Interface for the postprocessing module.
+
+Support postprocessing types:


I have added that we currently support these two types of postprocessing plugins.
We might need to add some tests for the ensemblestats type later.

Note that we have removed the ensemblestats type from the cookiecutter at the moment.

dnerini reviewed Jul 22, 2024

View reviewed changes

dnerini changed the title ~~Added structure for the creation of postprocessing plugins~~ Add diagnostic plugins Aug 5, 2024

dnerini mentioned this pull request Aug 5, 2024

Create ML-based nowcast plugin for pysteps #394

Open

ladc changed the title ~~Add diagnostic plugins~~ Adapt pysteps to allow for postprocessing plugins (diagnostics and ensemblestats for now) Aug 27, 2024

ladc changed the title ~~Adapt pysteps to allow for postprocessing plugins (diagnostics and ensemblestats for now)~~ Adapt pysteps to allow for postprocessing plugins Aug 27, 2024

ladc assigned joeycasey87 Aug 27, 2024

ladc requested a review from FelixE91 January 14, 2025 14:07

joeycasey87 and others added 14 commits January 14, 2025 15:14

Added structure for the creation of postprocessing plugins

f2c7514

Update interface.py

11abb0d

Updated to fit black formatting

36741db

Update interface.py

9e79882

Update diagnostics.py

5abe467

Remove redundant import and getattr statements

15de2a4

Adjusted so that the interface is more easily interpretable

c6ca85d

Fixed error from stashed chages

f5558ac

update test_pysteps

e64b70b

Changed the test file to match the test updated pysteps test file

Remove try-import statements for diagnostics.py

6605643

These were necessary for the IO plugins, but less so for the diagnostics.

Revert back to master and Python 3.10 in check_black.yml

2ed3411

Make sure the postprocessors are actually discovered in the main init

01bf7ba

Refactor code for postprocessing plugin detection.

3586f56

Avoid duplicate code, refactor into functions. Also fix a small typo causing a bug: postprocessor_s_

ladc force-pushed the postprocessor_plugin branch from d9b593d to 3586f56 Compare January 14, 2025 14:17

ladc added 2 commits January 28, 2025 17:56

Update dummy code names for new plugin structure

f581846

Change postprocessor interface to use diagnostic_ and ensemblestat_ p…

cb89616

…lugins

Felix Erdmann and others added 7 commits January 29, 2025 13:29

Fix the postprocessing interface

f9bf9cf

- should match names as in the plugin cookiecutter

Update postprocessing package to work with plugins

13922fc

- diagnostic plugins created with the cookiecutter are now correctly recognized and implemented

Update both io interface and postprocessign interface

cb2529c

- importer and diagnostic plugins correctly recognized in entry points - cleaning: removed unused import modules

Merge branch 'master' into postprocessor_plugin

c1c4817

Reformatted files with pre-commit

198df51

Merge branch 'postprocessor_plugin' of github.com:pySTEPS/pysteps int…

8c905f2

…o postprocessor_plugin

Remove tests for diagnostics plugins interfaces.

1305e88

ladc force-pushed the postprocessor_plugin branch from 51f9642 to 1305e88 Compare February 25, 2025 13:41

Felix Erdmann and others added 2 commits February 26, 2025 11:56

Fix io.interface to work with the new cookiecutter

296694d

Merge branch 'master' into postprocessor_plugin

b45c9a7

ladc self-requested a review February 26, 2025 14:38

ladc added 3 commits February 27, 2025 00:13

Test if the default cookiecutter plugin can be loaded in the CI tests.

a20ce17

Fix default plugin path in tox.ini.

a4e0b6a

Fix plugin tests to use new cookiecutter template; try out importer a…

f340085

…nd diagnostic plugin.

ladc reviewed Feb 26, 2025

View reviewed changes

Felix Erdmann added 7 commits February 27, 2025 13:00

Added postprocessing module interface test

06d181a

Postprocessing interface reformatted with pre-commit

17397e5

Cleaning as requested in ladc's review

f5df5a8

Fix postprocessing interface test

ccbb41f

Simplift postprocessing.interface and add more tests for it

db83773

Fix postprocessing.interface test to match expected warning

fa97a35

Bug fixes to run tests

444331a

more test - codecov

657e56e

FelixE91 reviewed Feb 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt pysteps to allow for postprocessing plugins #405

Adapt pysteps to allow for postprocessing plugins #405

joeycasey87 commented Jul 19, 2024

dnerini Jul 22, 2024

dnerini Jul 22, 2024

ladc Jul 22, 2024

codecov bot commented Jul 25, 2024 •

edited

Loading

joeycasey87 commented Aug 13, 2024

ladc commented Aug 20, 2024

dnerini commented Sep 8, 2024

ladc commented Jan 14, 2025 •

edited

Loading

ladc left a comment

ladc Feb 26, 2025

FelixE91 Feb 27, 2025 •

edited

Loading

ladc commented Feb 26, 2025

FelixE91 commented Feb 27, 2025

FelixE91 left a comment

FelixE91 Feb 27, 2025 •

edited

Loading

FelixE91 Feb 27, 2025

FelixE91 Feb 27, 2025

FelixE91 Feb 27, 2025

FelixE91 Feb 27, 2025

FelixE91 Feb 27, 2025

FelixE91 Feb 27, 2025

FelixE91 Feb 27, 2025


		Interface for the postprocessing module.

		Support postprocessing types:

Adapt pysteps to allow for postprocessing plugins #405

Are you sure you want to change the base?

Adapt pysteps to allow for postprocessing plugins #405

Conversation

joeycasey87 commented Jul 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 25, 2024 • edited Loading

Codecov Report

joeycasey87 commented Aug 13, 2024

ladc commented Aug 20, 2024

dnerini commented Sep 8, 2024

ladc commented Jan 14, 2025 • edited Loading

ladc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FelixE91 Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Footnotes

ladc commented Feb 26, 2025

FelixE91 commented Feb 27, 2025

FelixE91 left a comment

Choose a reason for hiding this comment

FelixE91 Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 25, 2024 •

edited

Loading

ladc commented Jan 14, 2025 •

edited

Loading

FelixE91 Feb 27, 2025 •

edited

Loading

FelixE91 Feb 27, 2025 •

edited

Loading