-
-
Notifications
You must be signed in to change notification settings - Fork 49
TraversalError in MultiplexedPath.joinpath when parent in compound path is missing #253
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
Comments
The exception callstack that should occur:
|
Hi @jaraco, is this something you could have a look at as you recently committed in this code? |
Yes. I'll take a look. As described, it's difficult for me to see what's going on, so I'll need to download and unzip the repro to investigate deeper. |
I've pushed the repro to the issue-253 branch for easier testing. |
It works fine for me:
|
Aha. So the issue seems only to arise only on older versions of Python.
|
A bit of debugging:
It appears from the code that hydra is looking for |
Looking at the calling code, it does appear as if the code is designed to trap exceptions when Perhaps the refactoring that happened in 5.8.0 changed the behavior around when an exception is raised. |
Here's what changed from 5.7.1 to 5.8.0. |
Can you confirm that this re-raise, which is new, is as intended? |
Aha. I see the issue. Previously, With 5.8 (#250 in particular), however,
The change was intended, but it wasn't necessarily intended that errors would be raised on To be sure Hydra could work around the issue by simply moving the |
I also want to figure out if the issue affects Traversables other that MultiplexedPath. Edit: After review, I believe the issue only affects MultiplexedPath, because the other implementation that changed ( |
I've put together a PR that (a) captures the expectation on the prior release, (b) demonstrates that test fails on the current main, and (c) fixes the issue by removing the re-raise. I'm not yet certain that's the right approach. One of the things that makes this behavior interesting is that MultiplexedPath is being used at all. The purpose behind I'm uncertain whether this usage (either by hydra or the repro) is correct. What does it mean, for example, when Does On the other hand, if Regardless, I believe you've identified a real potential defect here and if FFY00 agrees in the review of the PR, this will be fixed in 5.8.1 (and pertinent Pythons where 5.8 was merged). I urge you to determine if hydra or the repro are doing something incorrect that can be improved. |
Some additional information. Documentation for hydra search path plugins can be found here. Notice especially this part:
The documentation seems to imply that sometimes you don´t want to treat it as packages though. There is an example provided by Hydra, which can be found here. Notice however that the separator in the example it is not separated by '.', but by '/'. As can be seen here. Furthermore the most nested directory which contains configuration, doesn´t have an I will check with someone of the hydra developer team whether they can give some input. |
I asked a question to hydra maintainers here |
Uh oh!
There was an error while loading. Please reload this page.
I have a regression between version 5.7.1 and 5.8.0. I think the cause is this commit.
I have a reproduction scenario in the zip.
To reproduce:
If you change the setup.cfg to use version 5.7.1, no error is raised
repro.zip
The text was updated successfully, but these errors were encountered: