Skip to content

forecast.get_data doesn't allow extra kwargs #745

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

Closed
odow opened this issue Jul 2, 2019 · 1 comment · Fixed by #746
Closed

forecast.get_data doesn't allow extra kwargs #745

odow opened this issue Jul 2, 2019 · 1 comment · Fixed by #746
Milestone

Comments

@odow
Copy link
Contributor

odow commented Jul 2, 2019

Describe the bug

forecast.get_processed_data accepts **kwargs (good) and passes these to get_data (good) and process_data (good):

return self.process_data(self.get_data(*args, **kwargs), **kwargs)

but get_data doesn't allow superfluous keyword arguments (bad):
def get_data(self, latitude, longitude, start, end,
vert_level=None, query_variables=None,
close_netcdf_data=True):

Is the fix just to add **kwargs as the last argument to get_data? If so, you can assign this issue to me and I'll make a PR.

To Reproduce

import datetime
import pandas
from pvlib import forecast

forecast.NDFD().get_processed_data(
    38.238875, -117.363598, 
    pandas.Timestamp(datetime.datetime.now(), tz='US/Central'),
    pandas.Timestamp(datetime.datetime.now(), tz='US/Central') + pandas.Timedelta(hours = 12),
    how = 'liujordan'
)

Yields

Traceback (most recent call last):
  File "<stdin>", line 5, in <module>
  File "/anaconda3/envs/tonopah/lib/python3.7/site-packages/pvlib/forecast.py", line 306, in get_processed_data
    return self.process_data(self.get_data(*args, **kwargs), **kwargs)
TypeError: get_data() got an unexpected keyword argument 'how'

Expected behavior

I expect that get_processed_data is equivalent to:

model = forecast.NDFD()
data = model.get_data(
    38.238875, -117.363598, 
    pandas.Timestamp(datetime.datetime.now(), tz='US/Central'),
    pandas.Timestamp(datetime.datetime.now(), tz='US/Central') + pandas.Timedelta(hours = 12)
)
model.process_data(data, how = 'liujordan', extra_kwarg=False)

Note how I can pass extra_kwarg without any problem.

Versions:

  • pvlib.__version__: 0.6.3
  • pandas.__version__: 0.24.2
  • python: 3.7.3
@wholmgren
Copy link
Member

Good catch. I'm surprised that we've not seen this until now.

I agree that adding **kwargs to get_data is the cleanest way forward. I try to avoid using kwargs in the pvlib API, but we're already using it extensively within forecast.py and I don't see another good option.

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

Successfully merging a pull request may close this issue.

2 participants