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

Exclude tests from package discovery #6552

Merged
merged 4 commits into from
Mar 7, 2023

Conversation

Armavica
Copy link
Member

@Armavica Armavica commented Mar 1, 2023

setuptools was now picking up the tests directory as an installable package and was installing it alongside pymc, which could conflict with any other package named tests.

This PR configures setuptools to exclude tests from the automated package discovery.

I took this opportunity to clean up some warning-generating lines in MANIFEST.in, and an unused option in setup.py.

Checklist

Major / Breaking Changes

  • ...

New features

  • ...

Bugfixes

  • ...

Documentation

  • ...

Maintenance

  • Exclude tests from the package discovery

@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #6552 (d107a39) into main (2f945bb) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6552      +/-   ##
==========================================
- Coverage   92.01%   92.01%   -0.01%     
==========================================
  Files          91       91              
  Lines       15114    15108       -6     
==========================================
- Hits        13907    13901       -6     
  Misses       1207     1207              
Impacted Files Coverage Δ
pymc/data.py 89.82% <100.00%> (-0.36%) ⬇️

@ricardoV94
Copy link
Member

ricardoV94 commented Mar 1, 2023

Can we just move the relevant test utils TestMatchesScipy into the pymc package like numpy.testing module and leave tests outside? Wondering if that could be easier?

@twiecki twiecki added this to the v5.1.0 milestone Mar 1, 2023
@Armavica
Copy link
Member Author

Armavica commented Mar 1, 2023

Can we just move the relevant test utils TestMatchesScipy into the pymc package like numpy.testing module and leave tests outside? Wondering if that could be easier?

Yeah, much easier indeed. I will do that then, thanks :)

@Armavica Armavica changed the title Allow tests to be exported as pymc.tests again Exclude tests from package discovery Mar 3, 2023
@Armavica Armavica marked this pull request as ready for review March 3, 2023 17:51
@Armavica
Copy link
Member Author

Armavica commented Mar 3, 2023

I repurposed this PR to only fix the spurious tests package installation so that we can re-release quickly: the pymc-experimental dependency is less urgent because they install from git, so they don't depend on a release.

@ricardoV94
Copy link
Member

Might have broken ReadTheDocs?

@ricardoV94 ricardoV94 requested a review from OriolAbril March 3, 2023 18:06
This seems to be a relic of the past when `tests/data` contained data
files.  It is not the case anymore.
@OriolAbril
Copy link
Member

Same issue on test_scores = pd.read_csv(pm.get_data("test_scores.csv"), index_col=0) line as in the other PR that moved tests

@Armavica
Copy link
Member Author

Armavica commented Mar 3, 2023

I fixed it. The problem was that pm.get_data was first looking inside tests/data, which used to contain example files. This directory doesn't exist anymore, since 759acb0, so I believe that the fallback (downloading from github pymc-examples) is now always activated since then. So I just removed the local check.

# because of an upload-size limit by PyPI, we're temporarily removing docs from the tarball.
# Also see MANIFEST.in
# package_data={'docs': ['*']},
include_package_data=True,
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this exclude other stuff like example NBs or csv files?

Copy link
Member Author

@Armavica Armavica Mar 5, 2023

Choose a reason for hiding this comment

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

There is no more csv files in pymc (I think they moved to pymc-examples), and if NBs were ever included, I don't think they are not anymore. As far as I can say, building pymc with or without this line creates the same wheel.

@twiecki twiecki merged commit 534a9ae into pymc-devs:main Mar 7, 2023
@twiecki
Copy link
Member

twiecki commented Mar 7, 2023

Thanks for the quick turn-around @Armavica! Should we release 5.1.1 now?

@ricardoV94
Copy link
Member

@twiecki let's add back the test utilities that are used by other packages first. I'll push a PR now

@Armavica Armavica deleted the packaging-nosrc branch October 7, 2024 14:58
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.

4 participants