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

add test for require_variable() #343

Merged
merged 7 commits into from Mar 7, 2020
Merged

Conversation

jkikstra
Copy link
Collaborator

@jkikstra jkikstra commented Mar 6, 2020

This PR:

  • tests the behaviour of using a list of years as an argument for require_variable()

  • clarifies the docstring about the expected behaviour

  • Tests Added

  • Documentation Added

- tests the behaviour of using a list of years as argument
- updates the docstring
pyam/core.py Outdated Show resolved Hide resolved
tests/test_core.py Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
pyam/core.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Mar 6, 2020

Coverage Status

Coverage remained the same at 85.645% when pulling 7d9c476 on require_variable-behaviour into ebe40a1 on master.

pyam/core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
Co-Authored-By: Daniel Huppmann <dh@dergelbesalon.at>
tests/test_core.py Outdated Show resolved Hide resolved
tests/test_core.py Outdated Show resolved Hide resolved
jkikstra and others added 3 commits March 6, 2020 16:32
Co-Authored-By: Daniel Huppmann <dh@dergelbesalon.at>
Co-Authored-By: Daniel Huppmann <dh@dergelbesalon.at>
Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

thanks @jkikstra for improving the docs and adding a test!

food for thought - if you think that requiring a variable to exist for ALL years is a frequent use case, we might want to add a method=['any', 'all'] kwarg to the function in a next PR.

@danielhuppmann danielhuppmann merged commit 93a0585 into master Mar 7, 2020
@danielhuppmann danielhuppmann deleted the require_variable-behaviour branch March 18, 2020 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants