-
Notifications
You must be signed in to change notification settings - Fork 20
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
Adding statistics to Raster #638
base: main
Are you sure you want to change the base?
Conversation
31c857d
to
2311ee6
Compare
actual_name = stats_aliases[normalized_name] | ||
return stats_dict[actual_name] | ||
else: | ||
logging.warning("Statistic name '%s' is not recognized", stat_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or raising an error here? Or is the reasoning for warning instead to accept potential typos in the statistic list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not raised an error here in order to obtain results if stats_name is a list and only one stat is not recognized. But it could be passed as a raise if needed
Amazing! 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can you add a note to the documentation?
- Can you add a test with an inconsistent stats name and check the log returned?
- Did you manage to get it running on xdem?
"sum": "Sum", | ||
"sumofsquares": "Sum of squares", | ||
"sum2": "Sum of squares", | ||
"percentile": "90th percentile", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's kind to consider the user, but it does make the documentation a bit more complex. We'll see if we leave that much flexibility.
with caplog.at_level(logging.WARNING): | ||
stat = raster.get_stats(stats_name="80 percentile") | ||
assert isnan(stat) | ||
assert "Statistic name '80 percentile' is not recognized" in caplog.text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adebardo There is already a test with an inconsistent stats name and log return checked here 😃
@adebardo I added a section about |
@vschaffn To coordinate the changes between GeoUtils and xDEM, I typically do a If you want to run the tests/docs on xDEM's CI too, you can uncomment this line here in your xDEM PR: https://github.com/GlacioHack/xdem/blob/29a8cf4c8979d51723fe8b5a5a86e1fd5ba7cb96/environment.yml#L23. And potentially specify the GeoUtils branch at the end of the github link following https://pip.pypa.io/en/stable/topics/vcs-support/ 😉 |
Resolves #602 GlacioHack/xdem#660
Description:
This pull request introduces enhancements to the statistics calculation methods in the project. The goal is to provide users with more flexible and comprehensive statistics functionality for raster data. The
get_stats
method has been created to:A list of aliases has been drawn up in order to be as flexible as possible when users request statistics.
Changes:
note : nmad can't be called directly from a raster because of circular imports, you have to go through an array like.
NaN
if the statistic required is not in the listUsage Examples:
Get all statistics:
Get a single statistic (e.g., 'mean'):
Get multiple statistics:
Using a custom callable statistic:
Tests
The following tests ensure that the new functionality in the
get_stats
method works as expected:Full Statistics:
mean
,median
,max
,min
,sum
, etc.) for a raster and ensures each statistic is present and notNone
.Single Statistic:
float
.Selected Statistics:
mean
,maximum
, andstd
. Additionally, it includes the use of a custom statistic function (percentile_95
), which computes the 95th percentile of the data.Unrecognized Statistics:
get_stats
returnNaN
and a warning is raised if the statistic requested by the users is not in the list of aliasesNMAD tests in test_stats:
scipy.stats.median_abs_deviation
Documentation
A section about how to use
get_stats
has been added to the raster class documentation.