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

support setups with all modules in src-directory #9673

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bablokb
Copy link

@bablokb bablokb commented Sep 30, 2024

In larger libraries with multiple modules it makes more sense to put all modules into a src-directory and don't scatter them in the top-level directory. Python setuptools explicitly supports this setup as a default (in this case, pyproject.toml does not have to specify the modules explicitly, but this is only a side note and not relevant here).

This PR changes tools/preprocess_frozen_modules.py to support this setup. The change is backward compatible.

@bablokb
Copy link
Author

bablokb commented Sep 30, 2024

Failure was with one board: timeout during upload of artifact. I don't think this has anything to do with my change.

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

are you contributing patches elsewhere for this? e.g., I don't think a circuitpython library organized this way will work with circuitpython-build-tools, so it wouldn't be possible to organize a library like this and place it in the community bundle.

(I don't think there's a written rule for this but my gut tells me that to submodule something into frozen modules in the core we would want it to be in a bundle as well)

@bablokb
Copy link
Author

bablokb commented Sep 30, 2024

are you contributing patches elsewhere for this?

Not yet, but this is planned. Before I can contribute the library, I need to make sure it can be added as frozen.

e.g., I don't think a circuitpython library organized this way will work with circuitpython-build-tools, so it wouldn't be possible to organize a library like this and place it in the community bundle.

A patch for the community-bundle/circup would certainly be next step if necessary.

(I don't think there's a written rule for this but my gut tells me that to submodule something into frozen modules in the core we would want it to be in a bundle as well).

You should also take a few things into account:

  • having a src-directory is explicitly supported by the large Python world. Porting of existing packages will be easier if the src-layout is supported. See for example https://setuptools.pypa.io/en/stable/userguide/package_discovery.html#src-layout: "This layout is very handy when you wish to use automatic discovery, since you don’t have to worry about other Python files or folders in your project root being distributed by mistake. In some circumstances it can be also less error-prone"
  • the PR is backward compatible and therefore does not hurt.
  • In contrast, the current tools/preprocess_frozen_modules.py depends on a number of assumptions that are not clearly specified, e.g. you would happily include py-files from a doc dir (you only exclude docs) and you cannot name a module utils because you exclude it while tools or inst_scripts are not on your list and therefore would automatically be included. These are all random choices where as an explicit src-directory makes things clear.

I think it is ok to use heuristics like the current script does (or the circuitpython-build-tools), as long as there is no pyproject.toml.

This PR just improves the heuristics by providing an alternative that is less error-prone.

@tannewt tannewt added the tooling label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants