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

feat: support InputFileType.Json and InputFileType.Dict #2165

Merged
merged 14 commits into from
Dec 15, 2024

Conversation

XieJiSS
Copy link
Contributor

@XieJiSS XieJiSS commented Nov 12, 2024

This PR fixes supports for InputFileType.Json and InputFileType.Dict.

RAW_DATA_TYPES is defined as:

RAW_DATA_TYPES: List[InputFileType] = [
    InputFileType.Json,
    InputFileType.Yaml,
    InputFileType.Dict,
    InputFileType.CSV,
    InputFileType.GraphQL,
]

Currently, the code handles them in a if input_file_type in RAW_DATA_TYPES: branch. Due to the else: branch being too loose, only InputFileType.CSV and InputFileType.Yaml can be correctly parsed. Other input data types will be treated as InputFileType.Yaml and hence triggering exceptions.

PR tested locally using the following code:

from pathlib import Path
from tempfile import TemporaryDirectory

from datamodel_code_generator import DataModelType, InputFileType, generate

with TemporaryDirectory() as temporary_directory_name:
    temporary_directory = Path(temporary_directory_name)
    output = Path(temporary_directory / "model.py")
    generate(
        { "foo": 1, "bar": { "baz": 2 } },
        input_file_type=InputFileType.Dict,
        input_filename="placeholder",
        output=output,
        output_model_type=DataModelType.PydanticV2BaseModel,
        snake_case_field=True,
    )
    model: str = output.read_text()

print(model)

Original traceback:

Traceback (most recent call last):
  File "~/Library/Caches/pypoetry/virtualenvs/parser-ycrm5vgb-py3.12/lib/python3.12/site-packages/datamodel_code_generator/__init__.py", line 375, in generate
    obj = load_yaml(
          ^^^^^^^^^^
  File "~/Library/Caches/pypoetry/virtualenvs/parser-ycrm5vgb-py3.12/lib/python3.12/site-packages/datamodel_code_generator/__init__.py", line 53, in load_yaml
    return yaml.load(stream, Loader=SafeLoader)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/Library/Caches/pypoetry/virtualenvs/parser-ycrm5vgb-py3.12/lib/python3.12/site-packages/yaml/__init__.py", line 79, in load
    loader = Loader(stream)
             ^^^^^^^^^^^^^^
  File "~/Library/Caches/pypoetry/virtualenvs/parser-ycrm5vgb-py3.12/lib/python3.12/site-packages/yaml/cyaml.py", line 26, in __init__
    CParser.__init__(self, stream)
  File "yaml/_yaml.pyx", line 288, in yaml._yaml.CParser.__init__
TypeError: a string or stream input is required

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "~/Code/python/parser/parser/script/test.py", line 8, in <module>
    generate(
  File "~/Library/Caches/pypoetry/virtualenvs/parser-ycrm5vgb-py3.12/lib/python3.12/site-packages/datamodel_code_generator/__init__.py", line 381, in generate
    raise Error('Invalid file format')
datamodel_code_generator.Error: Invalid file format

After applying this patch:

# generated by datamodel-codegen:
#   filename:  placeholder
#   timestamp: 2024-11-12T06:50:33+00:00

from __future__ import annotations

from pydantic import BaseModel


class Bar(BaseModel):
    baz: int


class Model(BaseModel):
    foo: int
    bar: Bar

Reference:

https://github.com/koxudaxi/datamodel-code-generator/blob/f5c8852176270a3f01eb78a116140ab8cf26f853/datamodel_code_generator/__init__.py#L355..L381

Copy link

codspeed-hq bot commented Nov 24, 2024

CodSpeed Performance Report

Merging #2165 will not alter performance

Comparing XieJiSS:patch-1 (209f4d8) with main (7782e51)

Summary

✅ 31 untouched benchmarks

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Nov 24, 2024

Looks like some tests are failing - will have a deeper look tomorrow.

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 3, 2024

I've submitted a new commit which should fix the failing test.

Copy link

codecov bot commented Dec 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7782e51) to head (209f4d8).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #2165   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           38        38           
  Lines         4288      4294    +6     
  Branches       988       990    +2     
=========================================
+ Hits          4288      4294    +6     
Flag Coverage Δ
unittests 99.67% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 14, 2024

hmmmmm... Should I modify test cases to trigger the raise Error('Invalid file format') branch (I don't think this can be done without explicit type casting to Any), or should I just add a # noqa instead?

@koxudaxi
Copy link
Owner

hmmmmm... Should I modify test cases to trigger the raise Error('Invalid file format') branch (I don't think this can be done without explicit type casting to Any), or should I just add a # noqa instead?

We should try to add the test case if it's not diffcult.

@koxudaxi
Copy link
Owner

@XieJiSS Will you write the unittest? or Should I pick up the task?
I would release a new version this weekend.

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 14, 2024

@XieJiSS Will you write the unittest? or Should I pick up the task? I would release a new version this weekend.

Roger that, I'm trying to add a new test. If it goes smoothly I'll add another commit in ~1 hour, and if I met difficulties I'll come back and report it here.

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 14, 2024

I've added a test for the direct dictionary input feature. My local pytest result shows all tests are passing / skipped.

@koxudaxi
Copy link
Owner

koxudaxi commented Dec 14, 2024

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 14, 2024

Hi @koxudaxi, I checked the codecov report and noticed that there's a "partial" warning flag for the Dict branch. I think my test case should have covered this branch - it tests direct dictionary input with input_file_type set to InputFileType.Dict. And there's already another test case which test InputFileType.Dict + path to file as input. IMO together they should have covered all possible cases of this branch? Not sure what I've missed here.

@koxudaxi koxudaxi merged commit b8a27e7 into koxudaxi:main Dec 15, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants