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

3.2.0release #230

Merged
merged 2 commits into from
Jun 3, 2024
Merged

3.2.0release #230

merged 2 commits into from
Jun 3, 2024

Conversation

newville
Copy link
Member

@newville newville commented Jun 2, 2024

We need to set the version in pyproject.toml too!

Also, apparently uncertainties.unumpy was not being installed -- how did tests pass?

@newville newville requested review from wshanks, andrewgsavage, jagerber48 and a team June 2, 2024 20:26
Copy link

codecov bot commented Jun 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.74%. Comparing base (a4a48b3) to head (fb6f150).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #230   +/-   ##
=======================================
  Coverage   95.74%   95.74%           
=======================================
  Files          12       12           
  Lines        1903     1903           
=======================================
  Hits         1822     1822           
  Misses         81       81           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wshanks
Copy link
Collaborator

wshanks commented Jun 2, 2024

Hmm, tool.setuptools.packages.find should recurse into sub packages. I think I compared the wheel file lists before and after the pyproject.toml change and they agreed (I can't check at the moment).

Regarding the tests, we don't test the packaging currently because the uncertainties directory is top level and we run the tests from the top level. So the tests would pass even if we did not install the package. We could adjust things to make sure the installed files are tested.

@newville
Copy link
Member Author

newville commented Jun 3, 2024

In a fresh devel environment (deleting the build folder), pip install . does not install the unumpy submodule. And then running the tests by hand failed.

@wshanks
Copy link
Collaborator

wshanks commented Jun 3, 2024

Could it be that you just did not have numpy installed because you did not do pip install .[arrays]? For me, pip install . does install uncertainties/unumpy in the site-packages directory and python -m build . does produce a wheel with uncertainties/unumpy.

@newville
Copy link
Member Author

newville commented Jun 3, 2024

I do have numpy installed.

 > rm -rf build 
 > rm -rf  $CONDA_PREFIX/lib/python3.11/site-packages/uncertainties*
 > pip install numpy
 > pip install ".[all]"
 > ls   $CONDA_PREFIX/lib/python3.11/site-packages/uncertainties
__init__.py   __pycache__   core.py       umath.py      umath_core.py

uncertainties.unumpy is not installed.

You could use pyproject.toml with

[tool.setuptools.packages.find]
include = ["uncertainties*"]
exclude = ["doc", "test"]

to find subpackages, but

[tool.setuptools.packages.find]
include = ["uncertainties"]

will not find subpackages.

Copy link
Collaborator

@wshanks wshanks left a comment

Choose a reason for hiding this comment

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

Ah, you're right! I hadn't tried cleaning my repository before. It must have only worked for me because of the build/ directory or some other left over metadata.

Reading the setuptools documentation again, I think it would be better to change include to uncertainties* and get rid of exclude but what is in this PR is fine.

@newville
Copy link
Member Author

newville commented Jun 3, 2024

@wshanks Thanks. In other projects, I have favored an explicit list of sub-packages to avoid mistakes. But, now that "tests" have moved out, I think "uncertainties*" would be OK.

@newville newville merged commit 5116331 into master Jun 3, 2024
18 checks passed
@wshanks
Copy link
Collaborator

wshanks commented Jun 3, 2024

This is such a small project that an explicit list seems fine. I will open a PR to ensure that the package as installed from the wheel is tested in CI. I can switch to an explicit list (without find) there.

@wshanks
Copy link
Collaborator

wshanks commented Jun 3, 2024

I opened #232 to make sure we test the installed version of the package.

@newville newville deleted the 3.2.0release branch June 7, 2024 12:49
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