-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix failing Meteonorm tests #2548
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
Conversation
demo_url = 'https://demo.meteonorm.com/v1/' | ||
return demo_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong with the original? For a single URL definition as the whole function, isn't it okay just to leave it as:
demo_url = 'https://demo.meteonorm.com/v1/' | |
return demo_url | |
return 'https://demo.meteonorm.com/v1/' |
Same for the demo_api_key
Maybe there's something about python (style?) here that I don't know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a matter of convenience when I was hacking around locally. With the line split into two, you can run the single-line manually that defines the variable and then run the content of a test function.
Tests are a bit messy to debug sometimes because of all the magic they employ.
@AdamRJensen are we confident that we should switch to the new values in this PR, or do we need to wait to hear back from Meteonorm about the change? |
That is hard to say. I suggest that we merge it before making a new release if we do not hear back from them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We heard back from Meteonorm that the data changes are intentional. Let's merge this and maybe consider different testing strategies later.
Test failures are unrelated (the python 3.9 issue which was fixed elsewhere). Thanks @AdamRJensen! |
docs/sphinx/source/whatsnew
for all changes. Includes link to the GitHub Issue with:issue:`num`
or this Pull Request with:pull:`num`
. Includes contributor name and/or GitHub username (link with:ghuser:`user`
).remote-data
) and Milestone are assigned to the Pull Request and linked Issue.The Meteonorm iotools tests recently started failing due to changes on their end.