Skip to content
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

API suggestion: explicit accepted kwargs rather than **kwargs #119

Open
spencerahill opened this issue Dec 23, 2019 · 1 comment
Open

API suggestion: explicit accepted kwargs rather than **kwargs #119

spencerahill opened this issue Dec 23, 2019 · 1 comment

Comments

@spencerahill
Copy link

I was trying to make FixedInsolation instances representing different latitudes (for the daily-averaged insolation on a given day of the year). Using the coszen I generated using the daily_insolation function, I initially generated these via FixedInsolation(domains=state.Ts.domain, coszen=coszen), which didn't produce an error. However:

>>> FixedInsolation(domains=state.Ts.domain, coszen=0.2).coszen
Field([1.])

FixedInsolation evidently doesn't actually do anything with coszen; one has to instead feed it e.g. S0=S0*coszen to get the desired insolation value. But because its constructor accepts any kwarg, this ended up being quite hard to track down.

This speaks to a broader suggestion: switch from implicit to explicit call signatures for all of climlab's external API. I.e. rather than **kwargs at the end of each __init__, explicitly specify those arguments that are supported.

In this case, for example, since coszen evidently isn't handled by FixedInsolation, then I would have found the problem immediately, getting an error when I try to pass a coszen=0.2 in the first place.

I know this would be a decent amount of work, and it's not a bug per se, so maybe file it (if you even agree with the premise) in the "someday, climlab 1.0" file :)

@brian-rose
Copy link
Collaborator

Thanks @spencerahill, I generally agree with your assessment. In general the widespread use of class inheritance and **kwargs within climlab really gets in the way of usability.

I think a major overhaul is coming, so I'll keep this open and maybe shoot you some suggestions for your feedback as I re-work the APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants