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

metadata parser wrongly adds current date to temporal metadata #1151

Closed
wingechr opened this issue Jan 17, 2023 · 4 comments · Fixed by #1155
Closed

metadata parser wrongly adds current date to temporal metadata #1151

wingechr opened this issue Jan 17, 2023 · 4 comments · Fixed by #1155
Assignees

Comments

@wingechr
Copy link
Contributor

steps to reproduce: create a table and upload metadata with temporal values that only contain the year:

from oep_client import OepClient
import os
cli = OepClient(default_schema="sandbox", token=os.environ["OEP_API_TOKEN"])
table = "test_bug_metadata_temp"
cli.create_table(table=table, definition={"fields": []})
cli.set_metadata(table=table, metadata={
    "temporal": {
        "timeseries": {
            "start": "2021"            
        }
    }
})

the resulting metadata on the platform looks like this:

{
  "id": "test_bug_metadata_temp",
  "keywords": [],
  "temporal": {
    "timeseries": {
      "start": "2021-01-17T00:"
    }
  },
  "metaMetadata": {
    "metadataVersion": "OEP-1.4.0",
    "metadataLicense": {
      "name": "CC0-1.0",
      "title": "Creative Commons Zero v1.0 Universal",
      "path": "https://creativecommons.org/publicdomain/zero/1.0/"
    }
  }
}

2021-01-17T00 is wrong here, the current date (01-17) was added

@wingechr
Copy link
Contributor Author

  • the problem comes from omi.dialects.oep.parser.parse_date_or_none()
  • this dateutil.parser to convert a string to a datetime:
>>> from dateutil.parser import parse
>>> parse('2012')
datetime.datetime(2012, 1, 17, 0, 0)
  • this is clearly not what we want. if the user just gives the year, it should just stay a year.

@jh-RLI , @Ludee: proposed solution: i don't see why omi has to parse a date/datetime string from json into a datetime object. all we want is to make sure that it is a valid date/time/datetime string that CAN be parsed if needed. so i suggest to check if the value is

  • None
  • a number (e.g. just a year or maybe a unix timestamp)
  • a string that has a valid format for date/datetime
  • otherwise raise Exception

but not actually convert it into a datetime object (because why?)

@wingechr
Copy link
Contributor Author

additionally: even if the value is a proper datetime string, the "validated" result is truncated! ("2021-01-17T00:")

@jh-RLI
Copy link
Contributor

jh-RLI commented Jan 17, 2023

If I understand correctly, it seems very strange that a python datuitls library with 2k stars produces this kind of strange results. But this string worked for me with the new version of metadata and omi dialect. As far as I remember, at least it was not truncated and matched the original - I will check again. There is also a missing test for this case. Anyway, are you sure we don't need to worry about the datetime string format? I think some external software tools may rely on this format.

@wingechr
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants