VIP icon indicating copy to clipboard operation
VIP copied to clipboard

ENH: remove `method` argument from OO methods

Open r4lv opened this issue 5 years ago • 3 comments

While working with HCIDataset.recenter, I was thinking about whether we are giving the user too many parameters at once.

HCIDataset.recenter aggregates the preproc/recentering.py::cube_recenter_* functions, and merges all keyword arguments from these functions into one single signature, which results in a large list. Which of these arguments are actually needed depend on the method parameter.

problem

So when a user wants to improve his recentering, he has to read through the docstrings to check which parameters belong to his method, and which parameters he has to ignore. This makes it difficult for the user to see what is available, as he is flooded with options.

proposition

We could make the actual recentering methods a "sub-method", so that the cube_recenter_* functions stay encapsulated, but are still accessible directly from an HCIDataset. Example:

ds = HCIDataset(...)

ds.recenter.radon(...)
# or
ds.recenter.satspots(...)

This keeps the tab completion of HCIDataset. "clean", but helps the user with precise documentation for that specific method.

r4lv avatar Sep 10 '18 08:09 r4lv

I'm not against it, do you have a preliminary implementation to check?

carlos-gg avatar Sep 10 '18 12:09 carlos-gg

Here is a Gist on how it could work. Binder

r4lv avatar Sep 10 '18 12:09 r4lv

I coincidentally saw that pandas is doing the same thing with their DataFrame.

After constructing a DataFrame object

data = {"a": [1,2,3], "b":[1.5, 1.6, 1.7]}
df = pandas.DataFrame(data)

one can do

df.plot.scatter()
df.plot.line()

.plot is actually a FramePlotMethods object, which lays outside of the DataFrame.

Additionally, for backwards compatibility with versions < 0.17, one can use the kind keyword of .plot for the same thing (which is what I wanted to avoid to keep the docstrings / number of parameters minimal):

df.plot(kind="scatter")
df.plot(kind="line")

r4lv avatar Sep 21 '18 13:09 r4lv