Migrate to a src-layout - #196
Conversation
This commit migrates this package to a src-layout, which has [a number of advantages](https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/). Another advantage is that this change requires fewer mentions of the package name in `tox.ini`, which means that as addons adopt this change, there will be fewer lines which must diverge among them. The disadvantage to merging this is that any new files in outstanding pull requests will need to have their locations updated before merging.
Coverage Report for CI Build 28464688763Coverage remained the same at 93.685%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Point ruff to `src/qiskit_addon_utils` so qiskit_addon_utils is classified as a first-party import, fixing the spurious I001 lint failures.
|
I think that the benefits of this change worth the small disadvantage of changing the current open PRs. |
mrossinek
left a comment
There was a problem hiding this comment.
This looks great, thanks 👍
Two comments:
-
I agree with @dekelmeirom that it would be great if we can find one working convention for pure-Python as well as mixed Rust+Python repositories. In the latter,
src/would more typically be the location of the Rust code. Inqiskit-fermionsI usepython/for the Python package andcrates/for the Rust crates (since I have more than one). I am not sure whether usingpython/here makes sense though or would be considered confusing 🤔 -
The other comment is w.r.t. the name of the test-folder. Here we use
testwhich technically conflicts with Python's historicaltestmodule. That is why I more commonly usetests(note the plural form). It's technically not the scope of this PR, but on the same topic so I figured I raise this now.
| only-include = [ | ||
| "qiskit_addon_utils", | ||
| packages = [ | ||
| "src/qiskit_addon_utils", |
There was a problem hiding this comment.
As part of this PR, we should also re-evaluate whether the following lines are necessary. If they are, the path should be updated.
qiskit-addon-utils/docs/conf.py
Lines 19 to 21 in 12769b5
There was a problem hiding this comment.
FWIW I tried to locally remove them in my qiskit-fermions clone and the docs still generate just fine! So your hunch with this no longer being required seems correct 👍
This commit migrates this package to a src-layout, which has a number of advantages. Another advantage is that this change requires fewer mentions of the package name in
tox.ini, which means that as addons adopt this change, there will be fewer lines which must diverge among them. In particular,tox.inino longer contains any references to the package name.The disadvantage to merging this is that any new files in outstanding pull requests will need to have their locations updated before merging.