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

dvc exp run --set-param: unable to parse python params.py #9309

Closed
leonardcser opened this issue Apr 5, 2023 · 4 comments
Closed

dvc exp run --set-param: unable to parse python params.py #9309

leonardcser opened this issue Apr 5, 2023 · 4 comments
Labels
A: params Related to dvc params bug Did we break something? p2-medium Medium priority, should be done, but less important

Comments

@leonardcser
Copy link

leonardcser commented Apr 5, 2023

Bug Report

Description

When using a python file for the parameters, DVC ignores the override params flag if the value contains underscores:

MYPARAM = 100000   # works
MYPARAM = 100_000  # does not work

DVC does not display any messages that this param has not been updated.

Reproduce

  1. Create params.py
  2. Add a param with underscore as a value (see example above)
  3. Run dvc exp run -S 'params.py:MYPARAM=10'

Expected

The expected result would be DVC updating the params.py file with the value 10, but it is left unchanged without any warning.

Environment information

Output of dvc doctor:

$ dvc doctor
DVC version: 2.52.0 (pip)
-------------------------
Platform: Python 3.11.2 on macOS-12.6.2-x86_64-i386-64bit
Subprojects:
	dvc_data = 0.46.0
	dvc_objects = 0.21.1
	dvc_render = 0.3.1
	dvc_task = 0.2.0
	scmrepo = 0.2.1
Supports:
	gs (gcsfs = 2023.3.0),
	http (aiohttp = 3.8.4, aiohttp-retry = 2.8.3),
	https (aiohttp = 3.8.4, aiohttp-retry = 2.8.3),
	s3 (s3fs = 2023.3.0, boto3 = 1.24.59)
Cache types: reflink, hardlink, symlink
Cache directory: apfs on /dev/disk1s5s1
Caches: local
Remotes: gs
Workspace directory: apfs on /dev/disk1s5s1
Repo: dvc, git
Repo.site_cache_dir: /Library/Caches/dvc/repo/b8e837d66d4510b7d7e38d810359f146
@aguschin aguschin added bug Did we break something? A: params Related to dvc params p2-medium Medium priority, should be done, but less important labels Apr 5, 2023
@aguschin
Copy link
Contributor

aguschin commented Apr 5, 2023

Thanks for reporting, @leonardcser. Reproduced this.

@dberenbaum, setting this as p2 since this is rarely used notation in python and there is a simple workaround. WDYT?

@aguschin
Copy link
Contributor

aguschin commented Apr 5, 2023

I'll see if this is easily fixable though. If this is something I can do in an hour or two, I'll give it a try.

@pmrowla
Copy link
Contributor

pmrowla commented Apr 5, 2023

@aguschin this is a known issue and is not supported. The python params implementation should really only be considered experimental and only works in a limited set of basic use cases

Closing as duplicate of #5777

@pmrowla pmrowla closed this as not planned Won't fix, can't repro, duplicate, stale Apr 5, 2023
@skshetry
Copy link
Member

skshetry commented Apr 5, 2023

If someone can propose a fix, we are happy to review the PR. But as @pmrowla said, we consider it experimental, and recommend using either of json/toml/yaml files for parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: params Related to dvc params bug Did we break something? p2-medium Medium priority, should be done, but less important
Projects
None yet
Development

No branches or pull requests

4 participants