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

ValueError if parse_date gets none valid input #12

Closed
4lm opened this issue Jul 9, 2019 · 3 comments
Closed

ValueError if parse_date gets none valid input #12

4lm opened this issue Jul 9, 2019 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@4lm
Copy link
Contributor

4lm commented Jul 9, 2019

Hi all,

I have around 30 JSON files I try to convert from OEP-1.3 to OEP-1.4.

Around ten files are failing, because they have in temporal.reference_date a none valid input (string: "none" instead of a datestring) and produce an error ("Unknown string format:", timestr).

If I have a look at the corresponding file [parse.py] (https://github.com/OpenEnergyPlatform/omi/blob/dev/src/omi/dialects/oep/parser.py) I can see that there are multiple instances in which parse_date is used without a safety net.

I would solve the issue in the concerning code as follows. Also for all the other occurrences of parse_date in parse.py.

BEFORE:

        # filling the temporal section
        temporal = structure.Temporal(
            reference_date=parse_date(json_old["temporal"].get("reference_date")),
            start=None,
            end=None,
            resolution=None,
            ts_orientation=None,
        )

AFTER:

        # filling the temporal section
        try:
            temporal__reference_date = parse_date(json_old["temporal"].get("reference_date"))
        except ValueError:
            temporal__reference_date = json_old["temporal"].get("reference_date")
        temporal = structure.Temporal(
            reference_date=temporal__reference_date,
            start=None,
            end=None,
            resolution=None,
            ts_orientation=None,
        )

What do you think? Is this approach feasible?

@4lm 4lm added the bug Something isn't working label Jul 9, 2019
@MGlauer
Copy link
Contributor

MGlauer commented Jul 9, 2019

Yes, this is part of a larget issue, namely me not beeing aware of the fact that we do not have mandatory fields. I am working on it in features/everything_is_optional

@christian-rli
Copy link
Collaborator

Was this issue addressed in #13 ? I tried to parse a sample OEP-1.3 file after the merge and for testing purposes changed the date to "none". I got the following (same?) Error:

    raise ValueError("Unknown string format:", timestr)
ValueError: ('Unknown string format:', 'none')

@MGlauer
Copy link
Contributor

MGlauer commented Dec 6, 2019

Omi is able to parse the string

{
    "id": "id"
}

correctly in v1.3 and v1.4. Therefore I consider this closed

@MGlauer MGlauer closed this as completed Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants