Skip to content

Test examples #510

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

jrrickerson
Copy link

Begin work on #249 (and #248 to some degree) by adding a few Pytest-based unit tests to the "pure-hatch" example code.
I added some dummy data to use to test the actual "mean" calculations, since I thought a more complex mocking / faking example would probably be out of scope.

Moving forward, I'd like to add the tests to CI and add nox tests for the actual packaging steps.

As this is my first PR for the project, please let me know anything I might need to clean up!

jrrickerson and others added 3 commits May 21, 2025 11:51
This allows for src/package layout based tests to import the main
package during the unit tests.
Start testing example projects by adding Pytest tests to the Python code
itself.
Copy link
Collaborator

@ucodery ucodery left a comment

Choose a reason for hiding this comment

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

Thanks for starting these tests! They are going to be a big help to the guide.

I have opened a separate PR to your repo that addresses the knock-on effects of changing examplePy. There is not a good way to see this relationship or when it applies, so don't worry about it. Please merge that change before requesting a new review here.

from examplePy import temperature, temporal

# Get the path to the test data directory
TEST_DATA = pathlib.Path(__file__).parents[0] / "data"
Copy link
Collaborator

Choose a reason for hiding this comment

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

For a simple module in the wild, I would be okay with this load, but as we explicitly recommend Python's resources, can you please use importlib.resources to get this csv? https://docs.python.org/3/library/importlib.resources.html

You will have to add an __init__.py to the data dir.

Copy link
Author

Choose a reason for hiding this comment

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

Quick question - the CSV file is only really used to be able to test the example Python code. Do you still want it in the resources if it won't be included in the built package (due to the src/package example project layout)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The CSV file can be included, not in the build project, but in the sdist, with advanced Hatch build options.
https://hatch.pypa.io/1.12/config/build/#explicit-selection

However, I am okay opening a follow-up issue for this and doing it later. There is no urgency here, we are not publishing this package at all.

Probably want to reference/link to this discussion that involves test data
#59

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the clarification discussion and Hatch config links. I'll take a look at this and see if I can get it corrected, but I may be slightly delayed by travel.

@ucodery
Copy link
Collaborator

ucodery commented May 21, 2025

Summoning @lwasser: I thought we had recommended importlib.resources for exactly this scenario: 5kb of CSV files are used in testing. But our tests page Only recommends a "repository such as Figshare or Zenodo" and then using Pooch. But that seems like over kill for cases such as this. And although the package here is artificial, I don't think this case is very unusual.

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.

2 participants