Skip to content

Convert to src layout#7

Open
mdpiper wants to merge 1 commit into
mainfrom
mdpiper/use-src-layout
Open

Convert to src layout#7
mdpiper wants to merge 1 commit into
mainfrom
mdpiper/use-src-layout

Conversation

@mdpiper
Copy link
Copy Markdown
Member

@mdpiper mdpiper commented May 15, 2026

I asked Claude to convert the Python source in this repository from a single-file layout to a src layout. This is what it came up with:

  • Move diffusion_model.py into src/diffusion_model/__init__.py as an installable package
  • Extract the __main__ block into src/diffusion_model/__main__.py so python -m diffusion_model
    continues to work
  • Update pyproject.toml to use the setuptools packages.find discoverer pointed at src/, and
    add pythonpath = ["src"] for pytest
  • Add .venv/ to .gitignore

I think it's curious that Claude put all the functions into the module definition file __init__.py, but I guess it makes sense instead of having a named module like model.py and a stub definition file that imports functions from the named module file.

Move diffusion_model.py into src/diffusion_model/ as a package,
splitting the __main__ block into a separate __main__.py. Update
pyproject.toml to use the setuptools package finder for src/ and
add pytest pythonpath configuration.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mdpiper mdpiper marked this pull request as ready for review May 15, 2026 00:32
@mdpiper mdpiper requested a review from mcflugen May 15, 2026 00:32
@mcflugen
Copy link
Copy Markdown
Member

Move diffusion_model.py into src/diffusion_model/init.py as an installable package

I’d prefer not to move the implementation directly into src/diffusion_model/__init__.py.

I generally try to keep __init__.py as small as possible: version metadata, __all__, and maybe re-exporting public functions/classes from private modules. Putting much more than that there makes imports less predictable and blurs the distinction between package API and package internals. __init__.py can also quickly become a junk drawer.

Extract the main block into src/diffusion_model/main.py so python -m diffusion_model
continues to work

For python -m diffusion_model, I think __main__.py should also stay minimal, e.g.

from diffusion_model._cli import main

if __name__ == "__main__":
    raise SystemExit(main())

The CLI code itself should be wrapped in a main() function, even if it stays in the same module for now.

Update pyproject.toml to use the setuptools packages.find discoverer pointed at src/, and
add pythonpath = ["src"] for pytest

I’m also hesitant about adding this to pyproject.toml:

[tool.pytest.ini_options]
pythonpath = ["src"]

I’d rather have tests run against the installed package so packaging errors are caught during testing and adding src directly to PYTHONPATH can mask those issues.

Add .venv/ to .gitignore

Adding .venv/ to .gitignore is harmless, though maybe a little out of scope for this PR.

@mdpiper
Copy link
Copy Markdown
Member Author

mdpiper commented May 15, 2026

@mcflugen Thank you for reviewing this PR. Everything worked, but it had a bad smell. Would you like to make changes, or would prefer I do so?

@mcflugen
Copy link
Copy Markdown
Member

Would you like to make changes, or would prefer I do so?

You go for it!

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