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

Add complex TOML test data #98

Merged
merged 2 commits into from
Jun 1, 2022

Conversation

mondeja
Copy link
Contributor

@mondeja mondeja commented May 31, 2022

The unique problem that I see in the implementation is that JSON does not accept date types, so these must be defined as strings and checked using format. The solution that I see is to convert Python date objects into strings after parsing. Unfortunately, tomli does not offer an argument to convert date values as it does for floats.

I would appreciate some guidance for this task if you don't want to work on it.

Contributes to #95

@mondeja mondeja changed the title Add complex TOML test data case Add complex TOML test data May 31, 2022
@sirosen
Copy link
Member

sirosen commented May 31, 2022

@mondeja, thanks so much, this is awesome! It's more complete and more thorough than I was hoping.

I'd like to keep the "simple case" data, and I'll add handling to convert datetimes back into their string representations. So I'll make a commit on your PR branch which makes those changes -- plus gives you a shout-out in the changelog -- and assuming it passes CI I'll merge.

@sirosen sirosen merged commit 5fa9c47 into python-jsonschema:main Jun 1, 2022
@sirosen
Copy link
Member

sirosen commented Jun 1, 2022

I've just merged this along with some work to support TOML's first-class date and time types. It may be a few days before this is released, however. These changes revealed a bit of an issue with the way that format checkers were being built, and I need to think a little about whether or not I need to make additional changes.

@mondeja mondeja deleted the complex-toml-test-file branch June 1, 2022 08:06
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.

None yet

2 participants