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

Added JsonType to config #7475

Closed

Conversation

vladNed
Copy link
Contributor

@vladNed vladNed commented Sep 17, 2023

Change Summary

Added JsonType to be used for json_schema_extra property on ConfigDict

Related issue number

Closes #6348

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @sydney-runkle

@vladNed vladNed force-pushed the bug/modify-json-schema-extra-type branch from fec2a72 to 2595254 Compare September 17, 2023 20:12
@vladNed
Copy link
Contributor Author

vladNed commented Sep 18, 2023

please review

pydantic/config.py Outdated Show resolved Hide resolved
@hramezani
Copy link
Member

Thanks @vladNed for this.

Please fix the type for all json_schema_extra in the code base. You can find them by searching for json_schema_extra:.
for example:

json_schema_extra: dict[str, Any] | typing.Callable[[dict[str, Any]], None] | None

@hramezani
Copy link
Member

please update

@vladNed
Copy link
Contributor Author

vladNed commented Sep 19, 2023

@hramezani of course. I updated all json_schema_extra types

@vladNed
Copy link
Contributor Author

vladNed commented Sep 19, 2023

please review

@hramezani hramezani force-pushed the bug/modify-json-schema-extra-type branch from 6e4ba54 to 92d3712 Compare September 20, 2023 19:42
@hramezani
Copy link
Member

@vladNed I've pushed a commit and fixed the type

@vladNed
Copy link
Contributor Author

vladNed commented Sep 21, 2023

@hramezani looks good, still feels that doesn't approach the first suggested type in the story, as that was a recursive type. But.. if you find this will suite I feel that is more explicit this way.

@samuelcolvin
Copy link
Member

If we do this, (which I'm not sure is worthwhile):

  1. It should be call JsonValue - it's not a type
  2. It should be truly recursive.

Something like:

JsonValue: TypeAlias = Union[None, bool, int, float, str, List[JsonValue], Dict[str, JsonValue]]'

Please update.

@alexmojaki
Copy link
Contributor

@sydney-runkle can this be closed since it was replaced by #7803?

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

Successfully merging this pull request may close these issues.

ConfigDict.json_schema_extra has type Dict[str, object]
5 participants