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

Move package directory to project root, add LICENSE #4

Closed
wants to merge 1 commit into from

Conversation

metazool
Copy link
Contributor

Hi Lewis,

Thank you for setting up this python project template! We tried using it today while setting out some work on a new project https://github.com/NERC-CEH/object_store_api

As a result I've got some small suggested changes, quickly actionable:

  • Move the mypackage directory down into the repository root, in line with classic python patterns
  • Add a default LICENSE (chosen by looking through a selection of NERC_CEH repositories which include a licence

I can understand the .src/ simplifies the docker build but that shouldn't dictate the project layout - I left the src in the destination path in the Dockerfile (not easy to test without access to a VM that can run docker, though.

To test

git clone https://github.com/NERC-CEH/python-template.git
git checkout move_src_plus_license
./rename-package.sh whatever
grep -r whatever *

And eyeball the output... ideally also

docker build . -t whatever
docker run -it whatever

We had more questions about whether one can take a cookiecutter templated approach with Github repo templates (to avoid the potentially future-brittle bash/awk renaming) and hope to pick up good tips at the cookiecutter session at RSECon and add them to the discussion about project repo templates

@jmarshrossney also had a good suggestions about adding a tox setup to the CI/CD workflows - test multiple backends and versions while keeping it abstracted away from the Github Actions proprietary syntax for doing it.

I left the 3.12 requirement in the Dockerfile as is but set the minimum version in pyproject.toml down to 3.9 - this is motivated by research projects' tendency to have complex and not regularly maintained dependencies that require pinning to older python versions - alan-turing-institute/plankton-cefas-scivision#5 is a recent example...

Thank you again for setting this up, hopefully by trying to work with it we can make it more useful :D

@metazool metazool requested review from lewis-chambers and a team August 13, 2024 14:02
@lewis-chambers
Copy link
Collaborator

Hi Jo, thanks for making a PR, I agree with many of your points. It is good to have a default license, making a cookiecutter repo would be helpful for more dynamic usage of the template, and tox could indeed simplify migrations to other CI/CD providers. I'm sure the cookiecutter setup will be done eventually, but we decided that for the FDRI dev team it was better to have a "good enough" template and get started with development so we stopped short of setting up anything like that.

The src layout however is not related to the docker set up. It's a common Python layout that moves all importable code into the src directory and prevents import related bugs during development, more info here.

@metazool
Copy link
Contributor Author

So "src layout" vs "flat layout" does have benefits but is also a matter of personal taste - it's likely that my norms are a bit out of date! I had a quick look at half a dozen major libraries that I typically work with and they all use flat layout (some reserving src for C bindings) but these recommendations from pyopensci recommend adopting src for new projects, for cleanliness

https://www.pyopensci.org/python-package-guide/package-structure-code/python-package-structure.html

Maybe i'll withdraw this PR and introduce one just with the LICENCE, but still think you should consider a range of opinions while introducing a proposed template if you want to ensure it has takeup (I know you did offer a presentation on it recently).

While i'm here, is there a meaningful reason the project is private?

@metazool
Copy link
Contributor Author

Taking this back after reading up more on the benefits of adopting "src layout" for modern python, thank you @lewis-chambers

@metazool metazool closed this Aug 13, 2024
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