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 run: date values are not treated as strings #5868

Closed
phipsgabler opened this issue Apr 23, 2021 · 5 comments · Fixed by #8067
Closed

exp run: date values are not treated as strings #5868

phipsgabler opened this issue Apr 23, 2021 · 5 comments · Fixed by #8067
Labels
A: experiments Related to dvc exp bug Did we break something? p3-nice-to-have It should be done this or next sprint research

Comments

@phipsgabler
Copy link

Bug Report

Description

Parameters to be stored in a JSON file that are ISO dates (yyyy-mm-dd) are automatically treated as date values by exp run, it seems -- which then errors when writing them to JSON.

Reproduce

Clone this test repo and try the following:

$ dvc repro dostuff --force  # originally, the parameter is "2021-01-01"
Running stage 'dostuff':                                              
> python dostuff.py
Param is: 2021-01-01
Use `dvc push` to send your updates to remote storage.  
              
$ dvc exp run -S params.json:testparam="2021" # set to something else -- works
Running stage 'dostuff':                                              
> python dostuff.py
Param is: 2021
Updating lock file 'dvc.lock'                                         
...

$ dvc exp run -S params.json:testparam="2021-02-01" # set to a date -- fails
ERROR: unexpected error - Object of type date is not JSON serializable
...

Both 2021 and 2021-02 work and are parsed as an integer and a string, respectively.

Expected

Things should not be parsed as dates when I didn't say so; or at least, if they are parsed, this should work. Otherwise, I'd expect everything to be passed as an uninterpreted string.

Environment information

Output of dvc doctor:

DVC version: 2.0.18 (deb)
---------------------------------
Platform: Python 3.8.9 on Linux-5.4.0-72-generic-x86_64-with-glibc2.4
Supports: All remotes
Cache types: <https://error.dvc.org/no-dvc-cache>
Caches: local
Remotes: None
Workspace directory: ext4 on /dev/mapper/vg0-root
Repo: dvc, git
@pared
Copy link
Contributor

pared commented Apr 24, 2021

Seems that even "normal" run suffers from this problem:

#!/bin/bash

rm -rf wspace
mkdir wspace

pushd wspace

set -ex

main=$(pwd)
mkdir repo
pushd repo

git init --quiet
dvc init --quiet

echo "date: 2020-02-01" >> params.yaml

git add -A
git commit -am "init"

dvc run -n train -p date -o out "cp params.yaml out"

results in:

ERROR: unexpected error - object of type 'datetime.date' has no len(│
) 

@pared pared added bug Did we break something? p2-medium Medium priority, should be done, but less important labels Apr 24, 2021
@efiop efiop added the research label May 18, 2021
@andreyveresov
Copy link

Any date or datetime inside params.yaml leads to the object of type 'datetime.date' has no len() error

@daavoo daavoo added the A: experiments Related to dvc exp label Oct 20, 2021
daavoo added a commit that referenced this issue Aug 1, 2022
- Overriding a config value : foo.bar=value
- Appending a config value : +foo.bar=value
- Appending or overriding a config value : ++foo.bar=value
- Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object

---

Breaking changes:

To modify a list, `foo[0]=bar` must now be passed as `foo.0=bar`.

Modifying a nested list inside a dictionary is not supported by omegaconf.

---

Closes #4883
Closes #5868
Closes #6129
daavoo added a commit that referenced this issue Aug 2, 2022
- Overriding a config value : foo.bar=value
- Appending a config value : +foo.bar=value
- Appending or overriding a config value : ++foo.bar=value
- Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object

---

Breaking changes:

To modify a list, `foo[0]=bar` must now be passed as `foo.0=bar`.

Modifying a nested list inside a dictionary is not supported by omegaconf.

---

Closes #4883
Closes #5868
Closes #6129
daavoo added a commit that referenced this issue Aug 4, 2022
- Overriding a config value : foo.bar=value
- Appending a config value : +foo.bar=value
- Appending or overriding a config value : ++foo.bar=value
- Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object

---

Breaking changes:

To modify a list, `foo[0]=bar` must now be passed as `foo.0=bar`.

Modifying a nested list inside a dictionary is not supported by omegaconf.

---

Closes #4883
Closes #5868
Closes #6129
daavoo added a commit that referenced this issue Aug 4, 2022
- Overriding a config value : foo.bar=value
- Appending a config value : +foo.bar=value
- Appending or overriding a config value : ++foo.bar=value
- Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object

---

Breaking changes:

To modify a list, `foo[0]=bar` must now be passed as `foo.0=bar`.

Modifying a nested list inside a dictionary is not supported by omegaconf.

---

Closes #4883
Closes #5868
Closes #6129
daavoo added a commit that referenced this issue Aug 5, 2022
- Overriding a config value : foo.bar=value
- Appending a config value : +foo.bar=value
- Appending or overriding a config value : ++foo.bar=value
- Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object

---

Breaking changes:

To modify a list, `foo[0]=bar` must now be passed as `foo.0=bar`.

Modifying a nested list inside a dictionary is not supported by omegaconf.

---

Closes #4883
Closes #5868
Closes #6129
daavoo added a commit that referenced this issue Aug 5, 2022
- Overriding a config value : foo.bar=value
- Appending a config value : +foo.bar=value
- Appending or overriding a config value : ++foo.bar=value
- Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object

---

Breaking changes:

To modify a list, `foo[0]=bar` must now be passed as `foo.0=bar`.

Modifying a nested list inside a dictionary is not supported by omegaconf.

---

Closes #4883
Closes #5868
Closes #6129
daavoo added a commit that referenced this issue Aug 5, 2022
- Overriding a config value : foo.bar=value
- Appending a config value : +foo.bar=value
- Appending or overriding a config value : ++foo.bar=value
- Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object

---

Breaking changes:

To modify a list, `foo[0]=bar` must now be passed as `foo.0=bar`.

Modifying a nested list inside a dictionary is not supported by omegaconf.

---

Closes #4883
Closes #5868
Closes #6129
@blakeNaccarato
Copy link
Contributor

Should I open a new issue and report it as a regression? I'm getting the error, ERROR: failed to reproduce 'pipeline': Object of type date is not JSON serializable even in DVC version 2.27.2 on Python 3.10.7 and Windows 10.

dvc doctor

DVC version: 2.27.2 (pip)
---------------------------------
Platform: Python 3.10.7 on Windows-10-10.0.19044-SP0
Subprojects:
        dvc_data = 0.10.0
        dvc_objects = 0.4.0
        dvc_render = 0.0.11
        dvc_task = 0.1.2
        dvclive = 0.10.0
        scmrepo = 0.1.1
Supports:
        gs (gcsfs = 2022.8.2),
        http (aiohttp = 3.8.1, aiohttp-retry = 2.8.3),
        https (aiohttp = 3.8.1, aiohttp-retry = 2.8.3)
Cache types: hardlink, symlink
Cache directory: NTFS on C:\
Caches: local
Remotes: gs
Workspace directory: NTFS on C:\
Repo: dvc, git

dvc.yaml

stages:
  pipeline:
    cmd: "python -m boilerdata.pipeline"
    deps:
      - "data/curves"
    params:
      - "src/boilerdata/config/axes.yaml":
      - "src/boilerdata/config/project.yaml":
      - "src/boilerdata/config/trials.yaml":
    outs:
      - "data/results"

src/boilerdata/config/trials.yaml

trials:
  # snip
  - date: 2022-09-14
    group: "control"
    rod: "R"
    coupon: "A0"
    joint: "none"
    comment: "A descriptive comment."
    good: true
  # snip

@dberenbaum
Copy link
Collaborator

Sorry, @blakeNaccarato , I think it was a mistake to close this one.

@dberenbaum dberenbaum reopened this Sep 20, 2022
@dberenbaum dberenbaum added p3-nice-to-have It should be done this or next sprint and removed p2-medium Medium priority, should be done, but less important labels Feb 6, 2023
@mattseddon mattseddon closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2024
@blakeNaccarato
Copy link
Contributor

blakeNaccarato commented Mar 6, 2024

I see this has been closed as not planned. The reasoning in #9473 (comment) goes into more detail (for future readers landing here). Reminds me of that old yarn, falsehoods programmers believe about time.

So yeah, be sure to stringify your datetimes in params.yaml!

Looks like the PR discussion also covers intent to clarify the error when a bare datetime is in your params.yaml, and to document the unsupported YAML datetime type clearly. That could probably go in this part of the docs, if it's not already planned to be documented elsewhere.

If you're gonna triage some issues as out of scope, datetime-related ones are probably some of the best candidates! 😅

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 bug Did we break something? p3-nice-to-have It should be done this or next sprint research
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants