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

exp: handle : and . in metrics/params #9877

Closed
dberenbaum opened this issue Aug 24, 2023 · 1 comment
Closed

exp: handle : and . in metrics/params #9877

dberenbaum opened this issue Aug 24, 2023 · 1 comment
Labels
A: experiments Related to dvc exp A: metrics Related to dvc metrics A: params Related to dvc params p3-nice-to-have It should be done this or next sprint

Comments

@dberenbaum
Copy link
Collaborator

dberenbaum commented Aug 24, 2023

: and . are both special characters for metrics and parameters. Should we disallow them in metrics/params names? Should we enable escaping or some way to specify them?

But my objection is more that this same problem affects every other command/flag combination that uses path:metric_or_param addressing (like exp run --set-param and probably also stage add --params). I would prefer to only have to fix this once, in a consistent and re-usable way. This PR is just patching over one symptom of a broader underlying problem

Originally posted by @pmrowla in #9819 (comment)

Also in the same vein, I'm not sure that we actually document that . is an invalid character in metric or parameter names, but for all intents and purposes we don't support it because we treat . as the dictionary level separator in metric/param addressing (even though it is valid to have string keys that contain . in yaml/json). It would be easier for us to just treat : the same way and disallow it entirely in metric/param names

Originally posted by @pmrowla in #9819 (comment)

@dberenbaum dberenbaum added p3-nice-to-have It should be done this or next sprint A: experiments Related to dvc exp A: params Related to dvc params A: metrics Related to dvc metrics labels Aug 24, 2023
@dberenbaum
Copy link
Collaborator Author

Given that it's only come up once AFAIR, setting this to p3.

@dberenbaum dberenbaum closed this as not planned Won't fix, can't repro, duplicate, stale Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: experiments Related to dvc exp A: metrics Related to dvc metrics A: params Related to dvc params p3-nice-to-have It should be done this or next sprint
Projects
None yet
Development

No branches or pull requests

1 participant