Skip to content

BLD: Test editable installations for PyWavelets in CI#702

Merged
rgommers merged 2 commits intoPyWavelets:mainfrom
agriyakhetarpal:editable-installations-ci
Apr 18, 2024
Merged

BLD: Test editable installations for PyWavelets in CI#702
rgommers merged 2 commits intoPyWavelets:mainfrom
agriyakhetarpal:editable-installations-ci

Conversation

@agriyakhetarpal
Copy link
Collaborator

@agriyakhetarpal agriyakhetarpal commented Feb 22, 2024

Closes #700. This PR introduces a flag in the Linux matrix setup to run an editable installation of PyWavelets and run the test suite with pytest.

I can rebase this branch and revisit this PR after mesonbuild/meson-python#569 is merged and meson-python==0.16.0 is out on PyPI – it shall fix the failing test as noted in #700 (comment).

rgommers
rgommers reviewed Feb 22, 2024
View reviewed changes
@rgommers
Copy link
Member

rgommers commented Feb 22, 2024

Thanks! I'll get your other PR merged first, since there is some overlap. And then you can rebase this on top (CI will also run automatically once you have a PR merged).

@agriyakhetarpal agriyakhetarpal force-pushed the editable-installations-ci branch from f5f14d1 to 559ef3c Compare February 22, 2024 15:06
@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented Feb 22, 2024

Thanks! I rebased on top of the changes from the previous PR. The failure is expected, of course.

@agriyakhetarpal agriyakhetarpal force-pushed the editable-installations-ci branch from 1f97532 to a4996fb Compare February 22, 2024 15:12
@agriyakhetarpal agriyakhetarpal force-pushed the editable-installations-ci branch 6 times, most recently from 6a0a43b to d06f627 Compare April 17, 2024 13:54
@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented Apr 17, 2024 •
edited
Loading

With meson-python v0.16.0 out, this is now ready for review and the test suite against editable installations is working. It has been added for the Linux job for now – we can add it to the macOS job too if needed, but I think testing in just one place is perfectly fine as well. I rebased the branches and have revised the commit history, it should be easier to review/squash.

Edit: put out a request for review, didn't realise I forgot to do that :)

@agriyakhetarpal agriyakhetarpal marked this pull request as ready for review April 17, 2024 13:58
agriyakhetarpal
agriyakhetarpal commented Apr 17, 2024
View reviewed changes
@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented Apr 17, 2024

From the workflow run: https://github.com/PyWavelets/pywt/actions/runs/8723177022/job/23930800559?pr=702#step:4:446

this line shows that the wheel being built is an editable one, but the wheel does not have the editable keyword after the version and before the ABI tag – and I think this is a bug, it is not conformant with PEP 660. This is not the case with setuptools and other build-backends (just verified with both a Hatch-based pure Python project and a setuptools + pybind11 compiled one). I'll file this on the meson-python issue tracker.

@agriyakhetarpal agriyakhetarpal changed the title [WIP]: test editable installations for PyWavelets in CI BLD: Test editable installations for PyWavelets in CI Apr 17, 2024
@agriyakhetarpal agriyakhetarpal mentioned this pull request Apr 17, 2024
Closed
@rgommers
Copy link
Member

rgommers commented Apr 18, 2024

It has been added for the Linux job for now – we can add it to the macOS job too if needed, but I think testing in just one place is perfectly fine as well.

Agreed, a single job is fine.

rgommers
rgommers reviewed Apr 18, 2024
View reviewed changes
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Looks pretty good, only two minor comments.

@agriyakhetarpal
CI, BLD: Run tests on Linux editable installations
0ac654e 
This commit performs the following changes:

1. Adds a concurrency key to reduce duplicate runs
2. Moves common environment variables outside of
individual jobs and into the workflow context
for Linux and macOS
3. Adds an extra option to run an editable installation
of PyWavelets under the `--no-build-isolation` flag
with all of the build-time dependencies installed
beforehand, ignoring previous installations as
necessary.

The `pytest --pyargs pywt` invocation is used to
run the test suite against the editable installation
which is similar to the rest of the checks.
@agriyakhetarpal
BLD: Bump lower bound for meson-python
ae5de42 
Since the mechanism for editable installations
was improved in mesonbuild/meson-python#569
and subsequently released in meson-python
v0.16.0, this bumps the minimum version for it
for use during compilation.
@agriyakhetarpal agriyakhetarpal force-pushed the editable-installations-ci branch from d06f627 to ae5de42 Compare April 18, 2024 16:36
@agriyakhetarpal
Copy link
Collaborator Author

agriyakhetarpal commented Apr 18, 2024 •
edited
Loading

Thank you for the review, @rgommers! I have addressed all requested changes and rebased (only because they were minor). Let's hope that CI passes as usual.

rgommers
rgommers approved these changes Apr 18, 2024
View reviewed changes
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM now, in it goes. Thanks @agriyakhetarpal, nice work!

@rgommers rgommers merged commit 96a65e1 into PyWavelets:main Apr 18, 2024
@rgommers rgommers added this to the v1.7.0 milestone Apr 18, 2024
@agriyakhetarpal agriyakhetarpal deleted the editable-installations-ci branch April 18, 2024 19:31
@agriyakhetarpal agriyakhetarpal restored the editable-installations-ci branch May 3, 2024 21:33
@agriyakhetarpal agriyakhetarpal deleted the editable-installations-ci branch May 3, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rgommers rgommers rgommers approved these changes

Assignees

No one assigned

Labels

testing

Projects

None yet

Milestone

v1.7.0

Development

Successfully merging this pull request may close these issues.

Bug: cannot run tests with an editable installation of pyWavelets, possible pytesttester test collection discrepancy?

2 participants

@agriyakhetarpal @rgommers