.sel() returning comparer instead of .sel_df()?

[Hendrik1987]

How about a .sel() method that returns a copy of the original comparer with reduced data instead of:
https://github.com/DHI/fmskill/blob/aa1e47d7d0d4d8887faca763270a360d9c4d7da7/fmskill/compare.py#L354

That would allow calls like cc.sel().skill(), cc.sel().scatter() or in the future cc.sel().spatial_skill() without copying all the selection arguments to each method that allows selection.

The dataframe could be still returned via cc.sel().all_df(). Or maybe it should then become cc.sel().to_dataframe().

Further methods can easily be added, as long as they all return a comparer. Example: cc.sel().add_domains().skill(), where add_domains() could be a method to assign data points to sub-domains (#12).

[jsmariegaard]

This sounds like a good idea. I think sel_df should be renamed to _sel_df() in that case as it's still used for filtering in the existing methods (such as skill, scatter, etc) - which (I think) is cheaper than constructing a complete Comparer object each time.

I guess you're thinking that the sel() method should have similar arguments as sel_df() has now, so you could do things like:

cc.sel(start='2017-2-1').skill()

or

cc.sel(model='SW_2').scatter()

I like also your idea of a to_dataframe() function which returns the all_df which is now a property (notice that a Comparar also has a df attribute, which is used to serve all_df when someone asks for it). So the method to_dataframe() could return different dataframes depending on the context e.g. a SingleObsComparer doesn't need to have an observation column and a PointComparer doesn't need to have x- and y- columns. But on the other hand, if we would like to be able to reconstruct the comparer with such a dataframe i.e. cc = ComparerCollection.from_dataframe(df), then we maybe need all the columns...

[Hendrik1987]

Exactly, the .sel() should have the arguments of .sel_df().

I will give it a go on the track compare spatial branch and you can have a look.

I would desire the same behavior as xarray sel():

import xarray as xr
ds = xr.Dataset()
ds.attrs = dict(foo='bar')
ds2 = ds.sel()
ds2.attrs = dict(new='attrs')
print(ds)
print(ds2)

Note that ds2 is initiated from scratch and changing ds2 leaves ds intact. xarray ensures this by constructing the new dataset again: https://github.com/pydata/xarray/blob/7e48aefd3fd280389dee0fc103843c6ad7561e2b/xarray/core/dataset.py#L2178

How should we take a deep copy? Something like this?

class BaseComparer:
...
def __copy__(self,deep=True):
   return deepcopy(self)

and then in sel() start with
new = self.__copy__
followed by all the required selections.

Or what is a tidy way? Or is a deep copy not desired?

[jsmariegaard]

I think it would be better to finish a first version of spatial_skill() PR first and then open a new PR with the sel() work. Unless you think spatial_skill() cannot be done without the sel() method?

[Hendrik1987]

I do a first spatial_skill() without any selection arguments then. .sel() would avoid to copy all of those from .skill() to spatial_skill().

[jsmariegaard]

See also comments on sel() in #33

[Hendrik1987]

Closing as covered in #33