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

general: switch from flat layout to src layout #414

Merged
merged 1 commit into from
Jan 24, 2025
Merged

Conversation

karlicoss
Copy link
Owner

this is long overdue and much friendlier to python tooling ecosystem

should be backwards compatible with existing editable installs, with a warning to reinstall properly

see #316

this is long overdue and much friendlier to python tooling ecosystem

should be backwards compatible with existing editable installs, with a warning to reinstall properly

see #316
@karlicoss karlicoss merged commit f8a55f7 into master Jan 24, 2025
13 checks passed
@karlicoss karlicoss deleted the src-layout branch January 24, 2025 01:18
@purarue
Copy link
Contributor

purarue commented Jan 24, 2025

Nice, will probably update my modules accordingly at some point.

I've been half understanding how all the python tooling has changed to (basically) not support editable installs, so to confirm, this is still a namespace package, but its no longer using editable to allow you to override.

Is there a strategy to allow overriding modules, how does discovery work for conflicting files, is it just the one you installed latest overrides the old files?

@karlicoss
Copy link
Owner Author

karlicoss commented Jan 24, 2025

Yep, still namespace package!

Nono, this would still be editable! If you encounter any problems with editable setup after this change, please let me know!

If anything this should work better with editable installs now (this is one of the reasons I did a change).
When non-src layout is used, you end up with something like this, where pth file contains the code for loading the dirty_finder.py

__editable___HPI_0_5_20241020_dev11_dirty_finder.py
__editable__.HPI-0.5.20241020.dev11+dirty.pth

since it's dynamic, it's hard for tooling (in particular for mypy) to pick it up

With src layout is used, there is only a static .pth file

$ cat __editable__.HPI-0.5.20241020.dev12.pth 
/path/to/hpi/src

So this makes it much easier to integrate with tooling without executing code dynamically.

Relevant discussions: python/mypy#13392 (comment) pypa/setuptools#3518

PatMyron added a commit to PatMyron/HPI that referenced this pull request Jan 29, 2025
@PatMyron PatMyron mentioned this pull request Jan 29, 2025
@purarue
Copy link
Contributor

purarue commented Feb 5, 2025

Gotcha, can confirm I it creates a file like __editable__.HPI-0.4.20231103.dev133.pth

as a temp fix I can prepend my own repos in that file:

... /Repos/HPI-personal
.... /Repos/HPI
.... /Repos/HPI-karlicoss/src

I suppose this would be something maybe I could edit reorder_editable to hack around (so that things like all.py and overrides work) like described in the issue, unless theres some new workaround that Im unaware of.

for now this is ok (will just not re-install all the time), but will try and figure out something else -- it may have to be a whole separate command from reorder_editable because I dont want to break backwards compatability for locating the editable files.

@karlicoss
Copy link
Owner Author

Just to confirm, it's no different from before src/ layout right?
I'd expect the __editable__*.pth files and their naming to stay the same, the only difference should be that there is no more __editable___*_dirty_finder.py

Currently I'm just using an extra __00_force_hpi_order.pth file like I describe here -- this way if I reinstall packages for whatever reason, the order is preserved (and I needed that before the src/ layout change too)
purarue/reorder_editable#2 (comment)

@purarue
Copy link
Contributor

purarue commented Feb 5, 2025

Yeah, I dont see any major breaks yet, ran a hpi doctor, my_feed index, ran my bleanser modules, nothing looks like its crashing.

I think most of the reorder_editable logic can remain the same, it may just be as easy of a fix of pointing it at a _00_force_reorder_ediable.pth file, it is a hack regardless, Im not trying to come up with an elegant solution.

@karlicoss
Copy link
Owner Author

karlicoss commented Feb 5, 2025

Yeah, I've been thinking about it the other day and tried something else as an experiment:

cat /hpi/src/my/__init__.py   # "main" hpi package
__path__[:] = ['/hpi_private/src/my', '/hpi_overlay/src/my', '/hpi/src/my']

And it seems to work as expected? Not sure if there is anything I'm missing, so will test it more.

  • instead of absolute paths perhaps it could contain package names, and extract actual absolute paths with pkgutil or something like that
  • instead of hardcoding it could be specified in config. The only difficulty is that loading the config itself might require the path to be set up correctly (e.g. mine is in hpi_private), so a bit of a recursion here.. Could be specified in a separate config, which is a bit annoying (but then the .pth file managed by reorder_editable is also a separate config, so perhaps it doesn't make it worse 🤷 )
  • kinda relevant to the above, I'm thinking now that maybe the stub config (https://github.com/karlicoss/HPI/blob/master/src/my/config.py) in the main package is a bit of a mistake. If the import order is wrong, instead of import error during import my.config, it will get loaded and might make the missing config harder to debug. Perhaps it would be better to extract into a separate package and only install during mypy checks on the CI.

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