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

Spatial and robust stats, intro documentation + Update of spatialstats.py with new scikit-gstat features #159

Merged
merged 120 commits into from
Aug 14, 2021

Conversation

rhugonnet
Copy link
Member

@rhugonnet rhugonnet commented Jun 27, 2021

EDIT PR ALMOST READY:

Partly resolves #152

Refactors

Added documentation

  • Added Introduction
  • Added Spatial statistics page
  • Added Robust statistics page
  • Added gallery example Non-stationarity in elevation measurement errors
  • Added gallery example: Spatial correlation in elevation measurement errors
  • Added structure for other pages: Bias corrections, Filters, all added to API

Added/Improved plotting functionalities

  • Added plot_1d_binning, plot_2d_binning for results of nd_binning
  • Improved plot_vgm for results of sample_multirange_variogram

Refined variogram sampling and implementation of random methods

Tests and fixes

  • Added more robust tests for all functions related to sample_multirange_variogram
  • Refined tests of fit_sum_variogram
  • Fixed some issues with patches_method, for it to run faster (@adehecq your convolution approach is probably still better, we should implement it!)

TODO (just thought about it):

  • Add random_state for patches_method
  • Improve tests for patches_method
  • Fix bug in histogram of plot_vgm when log scale is used.
  • Fix spatialstats.py for referencing code in Spatial statistics page.
  • Clarify run and nruns arguments of sample_multirange_variogram

@rhugonnet rhugonnet marked this pull request as draft June 27, 2021 14:28
Copy link
Contributor

@erikmannerfelt erikmannerfelt left a comment

Choose a reason for hiding this comment

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

This will be a fantastic resource!! I look forward to seeing the text be more integrated with figures, schematics and the xdem API. Let me know if there's something I can do!

docs/source/intro.rst Outdated Show resolved Hide resolved
docs/source/intro.rst Outdated Show resolved Hide resolved
docs/source/intro.rst Outdated Show resolved Hide resolved
docs/source/intro.rst Outdated Show resolved Hide resolved
docs/source/spatialstats.rst Show resolved Hide resolved
docs/source/spatialstats.rst Show resolved Hide resolved
docs/source/code/spatialstats.py Show resolved Hide resolved
docs/source/intro.rst Outdated Show resolved Hide resolved
@rhugonnet
Copy link
Member Author

Is it correct that this PR is linked to #152? This would mean that the issue gets closed when this PR is merged.

It replaces uses with the subsampling function in spatialstats.py, but maybe not in other modules.
@adehecq Where else were you thinking we should update the subsampling when you raised this issue?

@rhugonnet
Copy link
Member Author

99 commits! You can do it!!

It's done!! 🥳 🎈

@adehecq
Copy link
Member

adehecq commented Jul 30, 2021

Is it correct that this PR is linked to #152? This would mean that the issue gets closed when this PR is merged.

It replaces uses with the subsampling function in spatialstats.py, but maybe not in other modules.
@adehecq Where else were you thinking we should update the subsampling when you raised this issue?

The two lines were this needed replacement are mentioned in the issue. So you can check that both are ok now.

@erikmannerfelt
Copy link
Contributor

@rhugonnet, with this PR, there is now a minimum version requirement for scikit-gstat. Do you know which? If so, can you specify that in the environment file?

@rhugonnet
Copy link
Member Author

@rhugonnet, with this PR, there is now a minimum version requirement for scikit-gstat. Do you know which? If so, can you specify that in the environment file?

@erikmannerfelt Done!
If it's all good for you, I think we can merge this PR.
There's still some TODOs left in the documentation, but those can be added step by step in the future!

erikmannerfelt
erikmannerfelt previously approved these changes Aug 13, 2021
Copy link
Contributor

@erikmannerfelt erikmannerfelt left a comment

Choose a reason for hiding this comment

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

Great job!

examples/plot_nonstationary_error.py Outdated Show resolved Hide resolved
examples/plot_standardization.py Outdated Show resolved Hide resolved
examples/plot_standardization.py Show resolved Hide resolved
xdem/spatialstats.py Show resolved Hide resolved
@rhugonnet rhugonnet merged commit c907122 into GlacioHack:main Aug 14, 2021
@rhugonnet rhugonnet deleted the doc_spstats branch August 14, 2021 08:13
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.

Make use of subsampling function
3 participants