Skip to content

Conversation

@schuenke
Copy link
Collaborator

@schuenke schuenke commented Sep 4, 2024

With #195 we switched from setup.py to pyproject.toml, where we defined:

[tool.setuptools.packages.find]
where = ["pypulseq"]

This works fine when you install pypulseq from a local folder using pip install ., but not if you want to install it directly from GitHub using pip install git+https://github.com/imr-framework/pypulseq.

With the direct install from GitHub, no pypulseq package will be created, but only the modules in our pypulseq folder (like SAR, seq_examples, Sequence, ...) will be installed as packages.

For me, removing the 2 lines above solves the problem, which makes sense IMO, because this is meant to tell setuptools where to look for our package and not which folder IS our package. Setuptools detects the src-layout, where the pypulseq folder would be in a src folder (we should switch to this layout !!!) and the flat-layout (the one we are still using) automatically. This is why we don't have to specify where to look for our package at all.

@FrankZijlstra
Copy link
Collaborator

Not an expert on this, but looks fine. Surprising that the behaviour is different depending on the install options. Have you checked whether it affects packaging the pypi package?

@schuenke
Copy link
Collaborator Author

schuenke commented Sep 4, 2024

Not an expert on this, but looks fine. Surprising that the behaviour is different depending on the install options. Have you checked whether it affects packaging the pypi package?

I didn't test it on PyPi directly, but tried the build command locally and installed it from the generated wheel in a clean env. That worked. Therefore, I am pretty sure it will work with PyPi as well.

@btasdelen
Copy link
Collaborator

Yeah, sorry, apparently I misunderstood what it does, I thought it would add pypulseq as a package, too.

Would you like to convert to src hierarchy in this PR, or should we go ahead and merge?

@FrankZijlstra
Copy link
Collaborator

Moving everything to src might create big merge conflicts on the other PRs? I'd suggest to incorporate those first (and I have one more adding the sequence examples back to the sequence unit tests).

@schuenke
Copy link
Collaborator Author

schuenke commented Sep 5, 2024

I agree that we should merge this one as well as #190 and #200 first and then create a seperate PR for switching to the src-layout.

@schuenke schuenke merged commit 2f09cf6 into imr-framework:dev Sep 5, 2024
@schuenke schuenke deleted the fix_install branch December 11, 2024 11:49
@schuenke schuenke mentioned this pull request Mar 21, 2025
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.

3 participants