Closed DavidMStraub closed 6 years ago
Totals | |
---|---|
Change from base Build 33: | 0.05% |
Covered Lines: | 3656 |
Relevant Lines: | 3801 |
The distinction between _default_options
and _options
is not 100% clear to me.
So I would think that _default_options
is used to specify (in the code, not by the user) which options should be available and what default values they should have. On the other hand, _options
is used to store any changes to the default option values as done by the user. So this would make totally sense for me.
What is not clear to me is what the use of set_default_option
is. This method allows the user to change the _default_options
. I don't see a use case for this, as it should be enough for the user to set the _options
by using set_option
(while I think _default_options
should only be changed directly inside the code). set_default_option
would also allow the user to add new options to _default_options
that do not have any effect (bypassing _option_check_key
).
You probably have some use case in mind for set_default_option
, but like it is implemented at the moment it might be confusing for the user to decide whether to use set_default_option
or set_option
to actually change the option value. Thus, I would drop the set_default_option
method to make it clear that options should be changed by the user using set_option
.
Is there actually something like set_default_option
in pandas
and how is it used?
Sorry, I missed that set_default_option
is a class method. But perhaps in that case, one could call it something like with_default_option
since it can be used to construct Wilson instances with the specified default option. This would perhaps make the distinction from the set_option
method clearer and possibly avoid some confusion (that I had...).
Basically when set_default_option
is called, the default options are modified, which are a class property. This means that they will be changed for all future instances of Wilson
. Use case: say you want to perform your analysis always with a specific value of some input parameter. Rather than having to call set_option
on each and every instance after instantiating it, you could then simply call
import wilson
wilson.Wilson.set_default_option(...)
and no longer worry about it.
The problem I can see with this is that set_default_option
can also be called on the instance, not just on the class, which might indeed be confusing as it affects the class, not just the instance. Not sure how to prevent that ...
As implemented at the moment, calling set_default_option
not only changes the default option for all future instances of Wilson
, but also for all already defined instances. So this also changes the result of their get_option
method as long as their method set_option
has not been called before. I think this might be confusing, but could easily be prevented by calling set_option
on initialization.
Good point. At first I thought it would be better to also change existing instances, but this is indeed problematic since they will have cached results with the old default parameters. Good idea to call set_option
in init.
OK done. Can you please rebase #13 onto this branch?
OK let's work with this interface, the config validation can be added later.
Motivated by #13.
@peterstangl, the
set_option
/get_option
methods are motivated bypandas
... do you find the interface intuitive?