Skip to content

Conversation

Kache
Copy link
Contributor

@Kache Kache commented May 30, 2025

"Pathing up" via __file__ (e.g. ../) is fragile, so only do it in a
centrally-defined config.ROOT_DIR, and path from root everywhere else

Also fix improper Python exec-as-module vs exec-as-script in dodo.py.
Needing to modify sys.path in order to import is a sign of having used
the wrong variant. For readers, a rule of thumb:

  • python foo/bar.py - independent, only depends on contents of foo
  • python -m foo.bar - depends on project modules outside of foo

These changes also facilitate a future migration towards the cleaner
src layout

In the future, this can be enforced with custom pylint checks, which is
easier to add after migrating the project to use pyproject.toml
instead of requirements.txt

@github-project-automation github-project-automation bot moved this to Review for relevance in Newsletter content May 30, 2025
@Kache Kache force-pushed the centralize-filepaths branch 3 times, most recently from a219b24 to 1974a96 Compare May 30, 2025 20:02
"Pathing up" via `__file__` (e.g. `../`) is fragile, so only do it in a
centrally-defined `config.ROOT_DIR`, and path from root everywhere else

Also fix improper Python exec-as-module vs exec-as-script in `dodo.py`.
Needing to modify `sys.path` in order to import is a sign of having used
the wrong variant. For readers, a rule of thumb:
 * `python foo/bar.py` - independent, only depends on contents of `foo`
 * `python -m foo.bar` - depends on project modules outside of `foo`

These changes also facilitate a future migration towards the cleaner
[src layout](https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/)

In the future, this can be enforced with custom pylint checks, which is
easier to add after migrating the project to use `pyproject.toml`
instead of `requirements.txt`
@Kache Kache force-pushed the centralize-filepaths branch from 1974a96 to 6843255 Compare May 30, 2025 20:07
@Felienne Felienne requested a review from rix0rrr June 13, 2025 06:43
@AnneliesVlaar AnneliesVlaar moved this from Review for relevance to Not newsletter material in Newsletter content Jun 30, 2025
@rix0rrr rix0rrr force-pushed the main branch 6 times, most recently from 674bb9f to 207fc8b Compare July 14, 2025 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress
Status: Not newsletter material

Development

Successfully merging this pull request may close these issues.

1 participant