teemtee
Introduce `MultiplexedPath` for referencing files in multiple locations
Could we track why we need a custom Path object? Reading the docstring, isn't pathlib.Path.relative_to(walk_up=True) what is needed there?
Could we track why we need a custom
Pathobject? Reading the docstring, isn'tpathlib.Path.relative_to(walk_up=True)what is needed there?
It is. But that was added in Python 3.12.
To me, it's a matter of convenience. E.g. https://github.com/teemtee/tmt/pull/3263/files#diff-80e5ba103da9529bd832a9cb0ea2712ceeee94b3dd5f92fcc75ed492817366b7R48. I really don't understand the reasoning behind not adding support for "append" to write_text/write_bytes, or even dedicated methods. Python developers decided "who needs this can use open() + mode='a', it's easy" - yeah, it is, but suddenly, I must use open() that's no longer needed for anything, to do something that could have been easily provided by Path classes, just like writing and reading of text blobs... Anyway.
I guess the question is, where does it stand in your way, and why would you like to get rid of it? Maybe we can find some solution.
Primarily the issue is when iteracting with other modules that generate a pathlib.Path, e.g. importlib.resources. If at any point in the path construction chain there is a different pathlib.Path or tmt.Path, it will inevitable propagate outwards which is too fragile.
Primarily the issue is when iteracting with other modules that generate a
pathlib.Path, e.g.importlib.resources. If at any point in the path construction chain there is a differentpathlib.Pathortmt.Path, it will inevitable propagate outwards which is too fragile.
Fair enough. It's really a shame there is no pathlib factory we could tell to use our class throughout the whole standard library.
If we decide to drop the class, we would need to replace its methods, probably with some tmt.utils helpers. It will be a shame though, I feel that WRT readability, returning to relative_to(some_path) or unrooted(some_path) is much worse than some_path.unrooted() :/ But it's doable, sure, the class exists only to 1. "fix" relative_to() - why the hell they dropped the walk_up functionality just to add it after several releases :/ - and 2. for convenience, like append_text() - again, why, just why forcing users to use open() just for one kind of writing into a file...
Yeah, the OOP interface is much neater, and backports for stdlib can be quite tricky, since you cannot rely on isinstance since you have to check for both backport and stdlib variants.
In principle the current approach could still work, but we need to be confident that mypy and pyright are catching all type modifications and we need to be careful not to loosely type-hint, e.g. in #2946 we need to change it from Traversable to strictly tmt.Path | MultiplexedPath.
A wrapper around Path would be ok I guess, but we would probably need to generalize it for #2946 to add a MultiplexedPath variant also.
Summary from the hacking session:
- keep the
tmt.utils.Pathimplementation with our custom methods - limit this issue scope for the
MultiplexedPathuse case (e.g. sourcing json schema files from multiple different directories)