DHI / modelskill

Compare results from MIKE and other simulations with measurements
https://dhi.github.io/modelskill
MIT License
33 stars 8 forks source link

drop() method to drop obs/mod/variable (reverse sel) #451

Open jsmariegaard opened 1 month ago

jsmariegaard commented 1 month ago

pandas and xarray allow users to drop part of the dataset with a drop method - see e.g. https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.drop.html. Which is opposite to sel() but should have the same syntax. The drop method should return a new Comparer or ComparerCollection with all data except the dropped.

Two new methods should be added to Comparer and ComparerCollection, respectively, both named drop(). Add them below sel() method https://github.com/DHI/modelskill/blob/db2f7c2d785d36e7bcef0c88c2269cbf10002ac7/modelskill/comparison/_comparison.py#L838 and https://github.com/DHI/modelskill/blob/db2f7c2d785d36e7bcef0c88c2269cbf10002ac7/modelskill/comparison/_collection.py#L301

ryan-kipawa commented 1 month ago

Dropping models, obs, and variables seems straightforward. What's a bit less clear is dropping according to the other arguments of sel():

I started #460 where I'll add the possibility to drop obs, models, and variables. Do you think the other ones are equally as important to add?

ecomodeller commented 1 month ago

xarray.Dataset.drop discourages the use of drop in favor of drop_vars and drop_sel, probably because of the potential confusion of dropping both rows or columns with the same method.

jsmariegaard commented 1 month ago

I don't think you will need to drop time or area in the same way as specific modelresults or observations so let's just do obs, models, and variables. I would prefer to keep the name drop() though