-
-
Notifications
You must be signed in to change notification settings - Fork 420
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
BUG: fix broken packaging #327
base: master
Are you sure you want to change the base?
Conversation
This reverts commit ef7fe6b.
Even with this patch, this does not look correct as this will install a package named |
I agree. I mentioned this in the issue linked in the description (additional considerations section). But since it involves a more serious refactoring I was hoping to get the opinion from one of the maintainers before moving forward. |
@FriedrichFroebel what would you like me to name the package? I also move the tests into the package so that they can be run by a CI (need this to make q available in conda. See this issue: #322 ) |
Usually
This should not be required if you do proper packaging of source and binary distributions. The |
Putting the tests inside the src directory makes it easier to run the tests in the CI for conda forge. I can try to leave them as is but if it's not a big deal to you I would suggest leaving them inside. I think tests are nice when making a tool like this available (for example I discovered that q can break with Python >= 3.11). What's more, there is no binary for Linux which is my main motivation for making q available in conda. |
This is the layout I propose based on what I have seen on best practices, naming conventions, and ease of testing @FriedrichFroebel.
|
As already said and mentioned inside the pytest docs as well: This depends on the preferred distribution model and some other factors. I tend to lean towards a strict separation of binary distributions (wheels) with the actual application code only and a separate source distribution (sdist) with all the source files (including tests) to use for building own wheels, OS-specific packages etc. Shipping the tests inside a dedicated package makes the actual installation from wheels smaller as it does not have to ship the testing stuff - this is the preferred way for libraries which are used in larger projects etc. where we can assume that the package itself is well-tested in an isolated manner. Shipping the tests inside the actual package tends to bloat the wheels and only makes sense if you want to run the tests from within a larger application, which does not make much sense in most of the cases. |
@FriedrichFroebel I understand. I restored the original project structure. Let me know if there is anything else that doesn't check out. |
I believe that i'm using an old ubuntu version for the github actions build/test workflow, and therefore some of the workflow jobs are infinitely queued without running. I will try to push some changes to the github actions to see if i can make them run properly. |
Closes #326
See issue above for detailed explanation.