metrics: show: Fix show from subdir. #8152
Conversation
daavoo
force-pushed
the
metrics-show-subdir
branch
from
4a9d636 to
69060f5
Compare
|
Does it fix #8020? |
It does 😄 . I didn't notice that issue. I found the bug researching a discord question |
|
|
||
| res = {} | ||
| for metric in metrics: | ||
| rel_metric_path = os.path.join(relpath, *fs.path.parts(metric)) |
There was a problem hiding this comment.
fs.path.join? As I recall one of the problems is windows behavior. dvcfs utlizizes posixpath.
There was a problem hiding this comment.
Um. Some unit tests are failing with fs.path.join.
os.path.join did solve the original issue.
@pared I wonder if this would be a breaking change for windows 🤔 (and potentially vscode?) as we would be changing the returned path format. So anything relying on json output would get different keys after this merge
There was a problem hiding this comment.
My bad, metrics results are relying on os specific paths. I wonder if that won't be problematic in the future. In plots we are heavily relying on fs.path. This is inconsistent, we might need to consider unifying it at some point.
daavoo
force-pushed
the
metrics-show-subdir
branch
from
69060f5 to
86df4da
Compare
daavoo
force-pushed
the
metrics-show-subdir
branch
from
86df4da to
6f0e562
Compare
daavoo
force-pushed
the
metrics-show-subdir
branch
2 times, most recently
from
86df4da to
fb5ac28
Compare
daavoo
force-pushed
the
metrics-show-subdir
branch
from
fb5ac28 to
ccc3209
Compare
This was a regression introduced in #7712. Calling `repo.metrics.show` from a subdir stopped returning metrics because `fs.isfile(metric)` check returned false.
daavoo
force-pushed
the
metrics-show-subdir
branch
from
ccc3209 to
5241c63
Compare
3 participants
Closes #8020
This was a regression introduced in #7712.
Calling
repo.metrics.showfrom a subdir stopped returning metrics becausefs.isfile(metric)check returned false.