First draft of stationary extreme value analysis module
Pull Request Checklist:
- [ ] This PR addresses an already opened issue (for bug fixes / features)
- This PR fixes #xyz
- [x] (If applicable) Documentation has been added / updated (for bug fixes / features).
- There is an example notebook at docs/notebooks/extreme_value_analysis.ipynb
- [x] (If applicable) Tests have been added.
- All the main parameter estimation functions in parameterestimation.py have been tested in tests/test_extremes
- [ ] CHANGELOG.rst has been updated (with summary of main changes).
- [ ] Link to issue (:issue:
number) and pull request (:pull:number) has been added.
- [ ] Link to issue (:issue:
What kind of change does this PR introduce?
A new subpackage called extreme_value_analysis has been added. This subpackage is a python wrapper around the julia Extremes.jl package.
As I am leaving, all the information about what needs to be done to finish this project can be found on 'https://docs.google.com/document/d/1Jg5_xQoyyekLfg8sfDff5-XGAzomPE8hSmA_VJZAKMo/edit?usp=sharing' (in French). @ospinajulian will be taking care of the julia wrapper from now on!
Does this PR introduce a breaking change?
Not to my knowledge.
Other information:
Need to add juliacall to the testing environment on GitHub for tests to pass.
Check out this pull request on ![]()
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
Welcome, new contributor!
It appears that this is your first Pull Request. To give credit where it's due, we ask that you add your information to the AUTHORS.rst and .zenodo.json:
- [ ] The relevant author information has been added to
AUTHORS.rstand.zenodo.json.
Please make sure you've read our contributing guide. We look forward to reviewing your Pull Request shortly ✨
Really strange that test test_gpfit, test_gpfitpwm and test_gpfitbayes aren't passing because they pass without a problem on my machine. Investigating..
All tests and notebooks passed, however I suspect there is some flaky behavior. We should probably wait until the pipeline passes a few times in a row
For ReadTheDocs, it's failing because you need to specify whether you want docs/notebooks/extreme_value_analysis.ipynb in the documentation (add it to an index.rst) or not (add it to ignored in docs/conf.py).
First complete pipeline pass
@Zeitsperre @RondeauG @sebastienlanglois I’ve already done what Kamil mentioned was missing. Also, I included some new features from Extremes.jl (e.g., confidence interval for the return levels) and added some of the comments from this PR. There are still a few details to improve and some issues to fix to pass the test. Even so, I’d appreciate your feedback at this point to improve anything that still needs work.
Je ne suis pas allé entièrement en détail, mais à première vue ça me semble très clean.
On avait discuté de l'idée de peut-être rendre ça seamless avec
xh.frequency_analysis.local. Est-ce que ça te semble encore réaliste, maintenant que tu as regardé le code ? (Si oui, ça serait probablement dans une PR séparée).
Yes I think it's possible. My first idea would be to set the distributions and methods of Extremes.jl as default to return the confidence interval and the non-stationary results. If the user wants to include another combination of dist and method, the function can computes the parameters with local.fit() and return NaNs for the confidence intervals. For the non-stationary it would return NaNs if the user tries to use a dist that is not in Extremes.jl.
@ospinajulian This seems to be (more or less) working now.
The issues I can see are:
- The notebooks use a lot of RAM and time, more than is available to use in ReadTheDocs. We might need to constrain the examples a bit more.
- The Julia version used in the CI is the one installed on the VM (not via conda). I think there may be some permissions issues or environment settings set by
JuliaCallthat don't translate well toconda-based Julia installations. Oddly enough, this works fine locally. Caveat emptor. - The tests that require
Juliashould be marked as such (with an accompanying marker definition underpyproject.toml'stool.pytest.ini_options). This isn't the end of the world, though.
I'm pushing some final changes. The minimum that needs to be dealt with are the docs. Good work!
@ospinajulian The docs failures ([codeautolink.match_name]) are expected. I'll push a fix tomorrow!
@ospinajulian If this Pull Request is ready, I would click the "Ready for review" button and let people know.
There are a lot of changes here, so maybe mentioning the files that require more attention could help us be more strategic with the review.
@ospinajulian If this Pull Request is ready, I would click the "Ready for review" button and let people know.
There are a lot of changes here, so maybe mentioning the files that require more attention could help us be more strategic with the review.
The file that requires the most attention is the one importing Julia (julia_import.py), as well as the changes I made in environment.yml, environment-dev.yml, and pyproject.toml. I would also appreciate it if you could take a look to files that implement the wrapper (i.e., paramestimation.py and util.py).