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

fix data and _ mismatch line 169 #90

Merged
merged 1 commit into from
Jan 22, 2024
Merged

Conversation

docNord
Copy link
Collaborator

@docNord docNord commented Jan 19, 2024

Closes #89

@docNord docNord added the bug Something isn't working label Jan 19, 2024
@docNord docNord requested a review from mgcth January 19, 2024 19:16
@docNord docNord self-assigned this Jan 19, 2024
@mgcth
Copy link
Collaborator

mgcth commented Jan 20, 2024

How do you call

def get_data(

to provoke the error reported in #89?

I can only manage to provoke another error from some non-thorough testing

c.get_data(2, 6057, interpolate=0)

@docNord
Copy link
Collaborator Author

docNord commented Jan 21, 2024

How do you call

def get_data(

to provoke the error reported in #89?

I can only manage to provoke another error from some non-thorough testing

c.get_data(2, 6057, interpolate=0)

Hmm, that is odd. I now get an error from MetObs when calling any station. MetObs is complaining that "Datum" is not included in the output from the SMHI API.
I dowloaded a file manually to see if the format has changed somehow, but that does not seem to be the case. Any ideas on what is going on @mgcth ?

@docNord
Copy link
Collaborator Author

docNord commented Jan 21, 2024

Update:
It seems there is a different return format specifically for parameter 2 @mgcth . I propose that you approve this PR and then we open a new issue to solve that problem. You should be able to recreate the issue reported with this code:

from smhi import smhi
s=smhi.SMHI()
data, _ = s.get_data(8,180960,interpolate=40)

@mgcth
Copy link
Collaborator

mgcth commented Jan 21, 2024

I'd like to reproduce the error here before I approve. How should I call get_data to reproduce the issue in #89 which you are fixing here?

@docNord
Copy link
Collaborator Author

docNord commented Jan 21, 2024

Did you try the snippet in my previous comment @mgcth ?

@mgcth
Copy link
Collaborator

mgcth commented Jan 21, 2024

Sorry, missed it. Do we have tests for this function? Looks like we don't. I'm thinking, it's probably a good time to add a unit test for it here. Though, the function is somewhat complex and could be refactored.

Copy link
Collaborator

@mgcth mgcth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll approve it now, looks correct. A refactor and tests would be nice.

@docNord docNord merged commit fa605a8 into main Jan 22, 2024
12 checks passed
@docNord docNord deleted the hotfix/smhi_root_interpolation branch January 22, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

index error when using interpolate with get_data
2 participants