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

Pooch integration, mark online tests, better handle online-dependent documentation, distributed testing #212

Merged
merged 16 commits into from
Oct 16, 2024

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Oct 10, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been added.
  • CHANGELOG.rst has been updated (with summary of main changes).
    • Link to issue (:issue:number) and pull request (:pull:number) has been added.

What kind of change does this PR introduce?

  • Marks several tests reliant on external services (online).
  • Better typing in call signatures.
  • Removes several unused imports.
  • Added pytest-xdist to the development dependencies.
  • Adding in a pooch registry for verifying file integrity when fetching testing data.

Does this PR introduce a breaking change?

Yes. pytest-xdist is now a development dependency. By default, it is not being used, but it can be enabled by providing positional arguments to the pytest call.

Other information:

I've copied and slightly improved upon the new testing data system currently in #175. I'll handle the merge of the changes to there, but this current system is nearing finalization. The last thing that I'm waiting on are some changes from @RondeauG to xhydro-testdata so that I can make a dated tag that will become the default testing data branch for xhydro (updated whenever there are additions to xhydro-testdata). This will help prevent breakages from a moving main branch over there.

@Zeitsperre
Copy link
Collaborator Author

I need to convert the exiting pooch calls to the new system, but before that, the pooch-based setup code causing errors at the moment needs to be converted to pytest fixtures. Will tackle that tomorrow.

@sebastienlanglois
Copy link
Contributor

@Zeitsperre I have implemented the fix for the GIS module here : #213

@Zeitsperre Zeitsperre marked this pull request as ready for review October 11, 2024 20:22
@Zeitsperre Zeitsperre changed the title Mark online tests and better handle online-dependent documentation Mark online tests and better handle online-dependent documentation, distributed testing Oct 11, 2024
@Zeitsperre Zeitsperre changed the title Mark online tests and better handle online-dependent documentation, distributed testing Pooch integration, mark online tests, better handle online-dependent documentation, distributed testing Oct 11, 2024
This was referenced Oct 11, 2024
Copy link
Contributor

@sebastienlanglois sebastienlanglois left a comment

Choose a reason for hiding this comment

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

Impressive work, thanks for improving the quality of the codebase!

@github-actions github-actions bot added the approved Approved for additional tests label Oct 11, 2024
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added notebooks Run tests against notebooks docs labels Oct 15, 2024
Copy link
Collaborator

@RondeauG RondeauG left a comment

Choose a reason for hiding this comment

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

We'll see if the GIS notebook passes. Otherwise, we might need to run the outputs locally as was done before.

(Note that I didn't include that notebook in #211, since it wasn't run by RtD).

@Zeitsperre Zeitsperre mentioned this pull request Oct 16, 2024
Merged
3 tasks
@Zeitsperre Zeitsperre merged commit 9057e1a into main Oct 16, 2024
17 checks passed
@Zeitsperre Zeitsperre deleted the adjust-tests branch October 16, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests docs notebooks Run tests against notebooks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants