search

iterative / gto

🏷️ Git Tag Ops. Turn your Git repository into Artifact Registry or Model Registry.
https://dvc.org/doc/gto
Apache License 2.0
142 stars 16 forks source link

Allow caps in dirname #420

Closed dacbd closed 1 year ago

pmrowla commented 1 year ago

related: https://github.com/iterative/gto/issues/403

dberenbaum commented 1 year ago

See also https://github.com/iterative/studio/pull/7844.

LGTM. I added #424 on top of this to support the same in model names.

dberenbaum commented 1 year ago

@dacbd @pmrowla Product questions are resolved. Is there anything blocking marking this as ready and merging?

codecov-commenter commented 1 year ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Files Coverage Δ
gto/constants.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

:loudspeaker: Thoughts on this report? Let us know!.

dberenbaum commented 1 year ago

See https://github.com/iterative/dvc/issues/9821#issuecomment-1747461256. Should we force tags to lowercase?

pmrowla commented 1 year ago

Since the tags can include directory paths, forcing them to lowercase would break behavior if the user has capitalized directory names and they are on a platform with case-sensitive filesystems.

The main issue here is that the dir path part of the regex disallows a lot of valid path characters (which was raised in https://github.com/iterative/gto/issues/403#issuecomment-1734274130)