Update contributing guide #241
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #241 +/- ##
=======================================
Coverage 80.78% 80.78%
=======================================
Files 140 140
Lines 10625 10625
=======================================
Hits 8583 8583
Misses 2042 2042
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
dnerini
approved these changes
Oct 24, 2021
There was a problem hiding this comment.
This is a very useful refresh to our documentation, thanks a lot! It looks all very good to me.
Once merged, we should not forget to cherry-pick it to the pysteps-v2 branch.
Also, I wanted to mention that these contributing guidelines could very well become the core content of our upcoming pystpes shortcourse for developers at erad 2022 next year.
RubenImhoff
approved these changes
Oct 25, 2021
There was a problem hiding this comment.
Nice work, @aperezhortal! I totally agree with Daniele and I have only a couple of very minor points myself.
aperezhortal
added a commit
that referenced
this pull request
Oct 25, 2021
- Improve grammar. - Make the guide more concise. Remove unused/unnecessary rules. - Indicate more clearly which parts of the guidelines are inspired by other projects (before they were only mentioned at the end). - Change "Travis-CI" references by "GitHub Actions".
dnerini
added a commit
that referenced
this pull request
Jan 14, 2022
* First basic functions to implement STEPS blending * Add compute of blend means,sigmas and recompose * pysteps.io with xarray (#219) * Add xarray dependency * MCH importer returns an xarray Dataset * Remove plot lines * Remove import * Adapt readers to xarray format * Rewrite as more general decorator * Add missing metadata * Adapt io tests * Mrms bounding box (#222) * Fix bounding box coordinates * Add missing metadata * Import xarray DataArray Ignore quality field * Black * Do not hardcode metadata * Address review comments by ruben * Add a legacy option to the io functions A "legacy" options is added to revert back the importers and readers behavior to version 1. This is a temporary solution to allow the examples, and other functions, to run as usual (v1.*). Hopefully, this is will allow a easier transition into version 2 during the development process and will allow testing functions that were not updated to v2. * Fix compatibility problems with tests Many of the tests were updated to use the legacy data structures (v1). The tests that still contains issues, were tagged with a TODO message and they are skipped. This will allow incremental changes to be tested in the new v2. IMPORTANT: once the v2 branch is stable, we may remove the legacy compatibility from the code base and the tests. * Update dependencies * Ignore plugins test Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> * Add blend_optical_flow * changes to steps blending procedure - weights according to adjusted BPS2006 method * changes to blending procedures - adjust weights from original BPS2006 method * Determine spatial correlation of NWP model forecast * First attempt to make correlations and thus weights lead time dependent (in progress..) * Change back to original BPS2006 blending formulation and add regression of skill values to climatological values for weights determination * Reformat code with Black * Skill score script imports climatological correlation-values from file now * Small changes to skill score script * Add skill score tests and an interface * Add skill score tests and an interface * Small change to docstring * Bom import xarray (#228) * Add import_bom_rf3 using xarray * Add tests to xarray version * Fix mrms importer tests * Pass **kwargs to internal functions * Add nwp_importers to read bom nwp sample data * Add bom nwp data to source file * Add tests for bom_nwp reader * Fix pystepsrc Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> * Functions to store and compute climatological weights (#231) * Implement the functions get_default_weights, save_weights, calc_clim_weights. These functions are used to evolve the weights in the scale- and skill-dependent blending with NWP in the STEPS blending algorithm. The current weights, based on the correlations per cascade level, are regressed towards these climatological weights in the course of the forecast. These functions save the current and compute the climatological weights (a running mean of the weights of the past n days, where typically n=30). First daily averages are stored and these are then averaged over the running window of n days. * Add tests for pysteps climatological weight io and calculations. * Add path_workdir to outputs section in pystepsrc file and use it as a default path to store/retrieve blending weights. * Minor changes to docstrings, changes to skill scores and testing scripts * Completed documentation for blending clim module, cleanup. Co-authored-by: RubenImhoff <r.o.imhoff@live.nl> * Main blending module, first steps * Add simple tests * Minor changes to tester: velocity now based on rainfall field of NWP * Add utilities to decompose, store and load NWP cascades for use in blending (#232) * First version of NWP decomposition * Added saving to netCDF * Completed functions for saving and loading decomposed NWP data * Added example files for the decomposed NWP functions * Added compatibility with numpy datetime * Use default output path_workdir for tmp files in blending/utils.py. * Update documentation of NWP decomposition functions in utils.py Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> Co-authored-by: wdewettin <87696913+wdewettin@users.noreply.github.com> * Add importer for RMI NWP data (#234) Add importer for netcdf NWP data from RMI using xarrays. * Add test for RMI NWP data importer. * Add entry for RMI NWP data in pystepsrc. * Run black on everything: fix formatting. * Add KNMI Harmonie NWP netcdf importer and tests (#235) * Changes to v_models to make use of multiple timesteps. Changes in the velocity field over time in the NWP forecast will be taken into account now. * Fixes for KNMI importer: Add forgotten @postprocess_import() Don't call dropna on NWP data. * Avoid shadowing of pysteps.blending.utils by pysteps.utils * First attempt for probability matching and masking utility; part 1 * Changes to prob matching and masking methods; part 2 * Prob matching and masking changes; part 3. Ready for testing with real data from here on * Remove unnecessary print statements * Cleanup imports * More cleanup * Update docstrings * RMI importer for gallery example (will follow) * Reprojection functionality (#236) * Added Lesley's reprojection module to this branch * Added compatibility for three-dimensional xarrays * Add commentary to reprojection util * Changes to make reprojection of KNMI data possible * Changes after Daniele's review * Add dependencies * Changes to importers, see issue #215 * Add tests * Fix some issues * documentation * Fixes for tests * Set requirements again * Some fixes * Changes to nwp_importers after Carlos' response * Remove wrong example script * Remove rasterio dependencies from lists * First try to prevent testing error * Changes Daniele and fix knmi nwp importer * Add rasterio to tox.ini * Aesthetics * rasterio import test * Add rasterio to the test dependencies * Reset try-except functionality for rasterio import * Fix for failing test on windows python 3.6 * add importerskip rasterio Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * Fixes in nwp importers * Revert "Merge branch 'steps_blending' into pysteps-v2" (#239) This reverts commit 2c639f8, reversing changes made to bccb8fc. * Merge latest version pysteps-v2 into steps_blending branch (#237) * Update docstrings * More cleanup * Cleanup imports * Cleanup imports * More cleanup * Update docstrings * Update references Mention the work of Ravuri et al (2021, Nature) as an example of work using cGANs to generate ensembles * Clean up page * Reprojection functionality (#236) * Added Lesley's reprojection module to this branch * Added compatibility for three-dimensional xarrays * Add commentary to reprojection util * Changes to make reprojection of KNMI data possible * Changes after Daniele's review * Add dependencies * Changes to importers, see issue #215 * Add tests * Fix some issues * documentation * Fixes for tests * Set requirements again * Some fixes * Changes to nwp_importers after Carlos' response * Remove wrong example script * Remove rasterio dependencies from lists * First try to prevent testing error * Changes Daniele and fix knmi nwp importer * Add rasterio to tox.ini * Aesthetics * rasterio import test * Add rasterio to the test dependencies * Reset try-except functionality for rasterio import * Fix for failing test on windows python 3.6 * add importerskip rasterio Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * Revert "Merge branch 'steps_blending' into pysteps-v2" (#239) This reverts commit 2c639f8, reversing changes made to bccb8fc. Co-authored-by: ned <daniele.nerini@meteoswiss.ch> Co-authored-by: dnerini <daniele.nerini@gmail.com> Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * NWP skill calculation only within radar domain * Update docs * Add example for gallery examples * Fix docstrings example * Remove additional normalization step * Fixes for the tests * update docs * changes to post-processing rainfall field and docstrings * Update contributing guidelines (#241) - Improve grammar. - Make the guide more concise. Remove unused/unnecessary rules. - Indicate more clearly which parts of the guidelines are inspired by other projects (before they were only mentioned at the end). - Change "Travis-CI" references by "GitHub Actions". * Advect noise cascade * Allow for moving domain mask of extrapolation component * minor fixes * Linear blending (#229) * Implemented linear blending function * Added example file and test * Added compatibility for NWP ensembles The PR is ready to go. Making the code xarray ready will be done in a separate PR. Co-authored-by: RubenImhoff <r.o.imhoff@live.nl> * weights calculation adjustment outside radar domain if only one model present * allow for mirroring of advected noise cascade * implementation of weights following Seed et al. (2013) * Allow for decomposed NWP precip and NWP velocity fields: part 2 * Store decomposed fields with compression * changes after first review Daniele * Remove unnecessary print statement * fixes to blending utils and implementation of blending utils tests * remove unnecessary lines * Fix one time step shift of extrapolation skill prior to blending * minor changes to blending climatology, blending weights and remove path_workdir from pystepsrc * Make NWP forecast decomposition prior to blending function optional * Use pathlib * Extract methods * Minor changes to docstrings * Access climatological skill file for multiple NWP model and date string changes to prevent errors in blending.utils Co-authored-by: Carlos Velasco <carlos.velasco@bom.gov.au> Co-authored-by: ned <daniele.nerini@meteoswiss.ch> Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> Co-authored-by: Ruben Imhoff <Ruben.Imhoff@deltares.nl> Co-authored-by: Carlos Velasco <cvelascof@gmail.com> Co-authored-by: Lesley De Cruz <lesley.decruz+git@gmail.com> Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> Co-authored-by: wdewettin <87696913+wdewettin@users.noreply.github.com> Co-authored-by: Lesley De Cruz <lesley.decruz@meteo.be> Co-authored-by: dnerini <daniele.nerini@gmail.com>
dnerini
added a commit
that referenced
this pull request
Jan 14, 2022
* First basic functions to implement STEPS blending * Add compute of blend means,sigmas and recompose * pysteps.io with xarray (#219) * Add xarray dependency * MCH importer returns an xarray Dataset * Remove plot lines * Remove import * Adapt readers to xarray format * Rewrite as more general decorator * Add missing metadata * Adapt io tests * Mrms bounding box (#222) * Fix bounding box coordinates * Add missing metadata * Import xarray DataArray Ignore quality field * Black * Do not hardcode metadata * Address review comments by ruben * Add a legacy option to the io functions A "legacy" options is added to revert back the importers and readers behavior to version 1. This is a temporary solution to allow the examples, and other functions, to run as usual (v1.*). Hopefully, this is will allow a easier transition into version 2 during the development process and will allow testing functions that were not updated to v2. * Fix compatibility problems with tests Many of the tests were updated to use the legacy data structures (v1). The tests that still contains issues, were tagged with a TODO message and they are skipped. This will allow incremental changes to be tested in the new v2. IMPORTANT: once the v2 branch is stable, we may remove the legacy compatibility from the code base and the tests. * Update dependencies * Ignore plugins test Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> * Add blend_optical_flow * changes to steps blending procedure - weights according to adjusted BPS2006 method * changes to blending procedures - adjust weights from original BPS2006 method * Determine spatial correlation of NWP model forecast * First attempt to make correlations and thus weights lead time dependent (in progress..) * Change back to original BPS2006 blending formulation and add regression of skill values to climatological values for weights determination * Reformat code with Black * Skill score script imports climatological correlation-values from file now * Small changes to skill score script * Add skill score tests and an interface * Add skill score tests and an interface * Small change to docstring * Bom import xarray (#228) * Add import_bom_rf3 using xarray * Add tests to xarray version * Fix mrms importer tests * Pass **kwargs to internal functions * Add nwp_importers to read bom nwp sample data * Add bom nwp data to source file * Add tests for bom_nwp reader * Fix pystepsrc Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> * Functions to store and compute climatological weights (#231) * Implement the functions get_default_weights, save_weights, calc_clim_weights. These functions are used to evolve the weights in the scale- and skill-dependent blending with NWP in the STEPS blending algorithm. The current weights, based on the correlations per cascade level, are regressed towards these climatological weights in the course of the forecast. These functions save the current and compute the climatological weights (a running mean of the weights of the past n days, where typically n=30). First daily averages are stored and these are then averaged over the running window of n days. * Add tests for pysteps climatological weight io and calculations. * Add path_workdir to outputs section in pystepsrc file and use it as a default path to store/retrieve blending weights. * Minor changes to docstrings, changes to skill scores and testing scripts * Completed documentation for blending clim module, cleanup. Co-authored-by: RubenImhoff <r.o.imhoff@live.nl> * Main blending module, first steps * Add simple tests * Minor changes to tester: velocity now based on rainfall field of NWP * Add utilities to decompose, store and load NWP cascades for use in blending (#232) * First version of NWP decomposition * Added saving to netCDF * Completed functions for saving and loading decomposed NWP data * Added example files for the decomposed NWP functions * Added compatibility with numpy datetime * Use default output path_workdir for tmp files in blending/utils.py. * Update documentation of NWP decomposition functions in utils.py Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> Co-authored-by: wdewettin <87696913+wdewettin@users.noreply.github.com> * Add importer for RMI NWP data (#234) Add importer for netcdf NWP data from RMI using xarrays. * Add test for RMI NWP data importer. * Add entry for RMI NWP data in pystepsrc. * Run black on everything: fix formatting. * Add KNMI Harmonie NWP netcdf importer and tests (#235) * Changes to v_models to make use of multiple timesteps. Changes in the velocity field over time in the NWP forecast will be taken into account now. * Fixes for KNMI importer: Add forgotten @postprocess_import() Don't call dropna on NWP data. * Avoid shadowing of pysteps.blending.utils by pysteps.utils * First attempt for probability matching and masking utility; part 1 * Changes to prob matching and masking methods; part 2 * Prob matching and masking changes; part 3. Ready for testing with real data from here on * Remove unnecessary print statements * Cleanup imports * More cleanup * Update docstrings * RMI importer for gallery example (will follow) * Reprojection functionality (#236) * Added Lesley's reprojection module to this branch * Added compatibility for three-dimensional xarrays * Add commentary to reprojection util * Changes to make reprojection of KNMI data possible * Changes after Daniele's review * Add dependencies * Changes to importers, see issue #215 * Add tests * Fix some issues * documentation * Fixes for tests * Set requirements again * Some fixes * Changes to nwp_importers after Carlos' response * Remove wrong example script * Remove rasterio dependencies from lists * First try to prevent testing error * Changes Daniele and fix knmi nwp importer * Add rasterio to tox.ini * Aesthetics * rasterio import test * Add rasterio to the test dependencies * Reset try-except functionality for rasterio import * Fix for failing test on windows python 3.6 * add importerskip rasterio Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * Fixes in nwp importers * Revert "Merge branch 'steps_blending' into pysteps-v2" (#239) This reverts commit 2c639f8, reversing changes made to bccb8fc. * Merge latest version pysteps-v2 into steps_blending branch (#237) * Update docstrings * More cleanup * Cleanup imports * Cleanup imports * More cleanup * Update docstrings * Update references Mention the work of Ravuri et al (2021, Nature) as an example of work using cGANs to generate ensembles * Clean up page * Reprojection functionality (#236) * Added Lesley's reprojection module to this branch * Added compatibility for three-dimensional xarrays * Add commentary to reprojection util * Changes to make reprojection of KNMI data possible * Changes after Daniele's review * Add dependencies * Changes to importers, see issue #215 * Add tests * Fix some issues * documentation * Fixes for tests * Set requirements again * Some fixes * Changes to nwp_importers after Carlos' response * Remove wrong example script * Remove rasterio dependencies from lists * First try to prevent testing error * Changes Daniele and fix knmi nwp importer * Add rasterio to tox.ini * Aesthetics * rasterio import test * Add rasterio to the test dependencies * Reset try-except functionality for rasterio import * Fix for failing test on windows python 3.6 * add importerskip rasterio Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * Revert "Merge branch 'steps_blending' into pysteps-v2" (#239) This reverts commit 2c639f8, reversing changes made to bccb8fc. Co-authored-by: ned <daniele.nerini@meteoswiss.ch> Co-authored-by: dnerini <daniele.nerini@gmail.com> Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> * NWP skill calculation only within radar domain * Update docs * Add example for gallery examples * Fix docstrings example * Remove additional normalization step * Fixes for the tests * update docs * changes to post-processing rainfall field and docstrings * Update contributing guidelines (#241) - Improve grammar. - Make the guide more concise. Remove unused/unnecessary rules. - Indicate more clearly which parts of the guidelines are inspired by other projects (before they were only mentioned at the end). - Change "Travis-CI" references by "GitHub Actions". * Advect noise cascade * Allow for moving domain mask of extrapolation component * minor fixes * Linear blending (#229) * Implemented linear blending function * Added example file and test * Added compatibility for NWP ensembles The PR is ready to go. Making the code xarray ready will be done in a separate PR. Co-authored-by: RubenImhoff <r.o.imhoff@live.nl> * weights calculation adjustment outside radar domain if only one model present * allow for mirroring of advected noise cascade * implementation of weights following Seed et al. (2013) * Allow for decomposed NWP precip and NWP velocity fields: part 2 * Store decomposed fields with compression * changes after first review Daniele * Remove unnecessary print statement * fixes to blending utils and implementation of blending utils tests * remove unnecessary lines * Fix one time step shift of extrapolation skill prior to blending * minor changes to blending climatology, blending weights and remove path_workdir from pystepsrc * Make NWP forecast decomposition prior to blending function optional * Use pathlib * Extract methods * Minor changes to docstrings * Access climatological skill file for multiple NWP model and date string changes to prevent errors in blending.utils Co-authored-by: Carlos Velasco <carlos.velasco@bom.gov.au> Co-authored-by: ned <daniele.nerini@meteoswiss.ch> Co-authored-by: Andres Perez Hortal <16256571+aperezhortal@users.noreply.github.com> Co-authored-by: Ruben Imhoff <Ruben.Imhoff@deltares.nl> Co-authored-by: Carlos Velasco <cvelascof@gmail.com> Co-authored-by: Lesley De Cruz <lesley.decruz+git@gmail.com> Co-authored-by: Wout Dewettinck <wout.dewettinck@ugent.be> Co-authored-by: wdewettin <87696913+wdewettin@users.noreply.github.com> Co-authored-by: Lesley De Cruz <lesley.decruz@meteo.be> Co-authored-by: dnerini <daniele.nerini@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a small PR updating to the contributing guidelines.
Main changes