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.

Sign up for GitHub

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

Jump to bottom

Test test_missing.py without cftime installed #5556

Merged
merged 11 commits into from
Jul 1, 2021
Merged

Test test_missing.py without cftime installed #5556

merged 11 commits into from
Jul 1, 2021

Conversation

Illviljan
Copy link
Contributor

@Illviljan Illviljan commented Jun 30, 2021 •
edited
Loading

The checks that didn't install cftime skipped the entire test_missing.py. Because cftime is also imported when importing the list of calenders _CFTIME_CALENDERS.

Added also skips for the cases that assumed cftime was installed so they are correctly skipped.

  • Passes pre-commit run --all-files

@pep8speaks
Copy link

pep8speaks commented Jun 30, 2021 •
edited
Loading

Hello @Illviljan! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-06-30 21:11:02 UTC

@github-actions
Copy link
Contributor

github-actions bot commented Jun 30, 2021 •
edited
Loading

Unit Test Results

         6 files  ±0           6 suites  ±0   51m 45s ⏱️ ±0s
16 156 tests ±0  14 441 ✔️ ±0  1 715 💤 ±0  0 ❌ ±0 
90 132 runs  ±0  82 027 ✔️ ±0  8 105 💤 ±0  0 ❌ ±0 

Results for commit 0e7c92c. ± Comparison against base commit 0e7c92c.

♻️ This comment has been updated with latest results.

@Illviljan Illviljan mentioned this pull request Jun 30, 2021
Merged
2 tasks
@Illviljan Illviljan closed this Jun 30, 2021
@Illviljan Illviljan reopened this Jun 30, 2021
@max-sixty max-sixty added the plan to merge Final call for comments label Jun 30, 2021
dcherian
dcherian approved these changes Jun 30, 2021
View reviewed changes
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

I like this. Thanks!

@max-sixty
Copy link
Collaborator

Thanks @Illviljan !

@max-sixty max-sixty merged commit 0e7c92c into pydata:main Jul 1, 2021
@Illviljan Illviljan deleted the test_missing_without_cftime branch July 2, 2021 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcherian dcherian dcherian approved these changes

Assignees
No one assigned
Labels
plan to merge Final call for comments
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Illviljan @pep8speaks @max-sixty @dcherian