-
Notifications
You must be signed in to change notification settings - Fork 12
v2.0 rewrite: schema generation from Julia types #70
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
Conversation
This is a complete rewrite of JSONSchema.jl that transforms it from a
pure validation library to a schema generation and validation library.
## New Features
- `schema(T)`: Generate JSON Schema from Julia struct definitions
- `Schema{T}`: Type-parameterized schema for type-safe validation
- StructUtils integration for field-level validation tags
- `$ref` support for schema deduplication and recursive types
- `ValidationResult` struct with detailed error messages
## Breaking Changes
- `validate()` now returns `ValidationResult` instead of `nothing`/`SingleIssue`
- `JSON.schema/validate/isvalid` convenience methods removed
- `parent_dir` kwarg removed from `Schema` constructor
- See docs/src/migration.md for full migration guide
## Backwards Compatibility
- `schema.data` field access (maps to `schema.spec`)
- `Schema(true)`/`Schema(false)` boolean schemas
- `validate(data, schema)` inverse argument order
- `isvalid(data, schema)` inverse argument order
- `required` validation without `properties`
- `diagnose()` function (deprecated)
- `SingleIssue` type alias
## Other Changes
- Removed JSON3 extension
- Updated CI to modern GitHub Actions
- Added comprehensive documentation
- Bumped minimum Julia version to 1.10
- Version bump to 2.0.0
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
I don't necessarily object, but merging in 7 minutes without review is not good practice. |
|
What does "Generated with Claude Code" mean. To what extent? Why was this rewrite necessary? Who is the use-case for? This needed much more discussion before merging. |
|
Sorry, yeah, that was hasty of me. Too eager to get a release out for other downstream changes/dependencies. I was partly wondering if anyone would want to review such a big rewrite, but I should have at least let it stay open. I'm using claude mostly to help with actual package maintenance tasks:
The rewrite updates the spec version teh package supports (draft6 -> draft7), and includes new "schema generation" functionality that I need in other projects. It integrates with the StructUtils.jl package to utilize the "field tag" functionality where you can specify, per field, json schema properties/constraints/etc. I tried to do a fairly comprehensive review of the latest 1.5 release and add in a "compat.jl" layer ot make the 2.0 release as minimally breaking as possible, along with a "migration guide" for the few remaining minor breakign pieces. |
| Schema{T} | ||
| A typed JSON Schema for type `T`. Contains the schema specification and can be used | ||
| for validation via `JSON.isvalid`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON.isvalid doesn't exist. Did you mean JSONSchema.isvalid? Otherwise, why aren't we overloading Base.isvalid like we were previously? This break is a breaking change for JuMP.
| ```julia | ||
| schema = JSON.schema(User) | ||
| instance = User("alice", "alice@example.com", 25) | ||
| is_valid = JSON.isvalid(schema, instance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code isn't self-contained. What is User? Same issue with JSON/JSONSchema
| old_path = match(r"^(.*/).*$", uri.path) | ||
| if old_path === nothing | ||
| els[:path] = id2.path | ||
| # JSON Schema generation and validation from Julia types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I strongly prefer we retain the copyright headers at the top of files
| # Context for tracking type definitions during schema generation with $ref support | ||
| mutable struct SchemaContext | ||
| # Map from Type to definition name | ||
| type_names::Dict{Type, String} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a reason for removing JuliaFormatter?
|
|
||
| on: | ||
| push: | ||
| branches: [main, master] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main branch doesn't exist
| ], | ||
| ) | ||
|
|
||
| deploydocs(repo = "github.com/JuliaServices/JSONSchema.jl.git", push_preview = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is JuliaServices? Surely this needs to be JuliaIO?
|
|
||
| ## Quick Start | ||
|
|
||
| ```julia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strong preference that all code blocks in the documentation need to be @repl otherwise it's too easy for there to be a mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I'm not familiar with @repl; I'll study up and change.
| ### 1. `validate()` Return Type | ||
|
|
||
| **This is the main breaking change.** The `validate` function now always returns | ||
| a `ValidationResult` struct instead of `nothing` on success. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the motivation for this?
| **Migration:** Replace `validate(...) === nothing` with `validate(...).is_valid` | ||
| or use `isvalid(schema, data)` which returns a boolean. | ||
|
|
||
| ### 2. `JSON.schema`, `JSON.validate`, `JSON.isvalid` No Longer Available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this Claude? This is just wrong
| **v1.x:** | ||
| ```julia | ||
| using JSONSchema | ||
| JSON.isvalid(schema, data) # Worked via runtime registration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. It was Base.isvalid, or just even isvalid
|
I don't know how much of a review you want just yet. This needs a much more careful review before release. My main ask is that we use I'd also prefer that |
|
I have to admit, I don't use AI tools very much, and this experience doesn't make me want to. (I do use them to ask questions, etc, but not to write code.) |
|
I think my main concern with Base.isvalid was whether it was too much of a "pun" (since it's used for string validity?) but I guess if that's what was being used before, it's probably fine to keep. The reason for ValidationResult was so you could know: 1) is it valid and 2) if not, what were all the errors (instead of just throwing eagerly on the first encountered !isvalid). We could probably come up with a different way to support that though and maintain the Sorry for teh messiness; it's as much my fault here as the AI tool who tried its best. The issue is I originally had this code all in the JSON.jl package, but after a while, realized it should probably live as a stand-alone package. Then realized we already had a JSONSchema.jl package, and that's what led to the code moving here. And then I started moving the code a week or two ago and then tried to finish the move today and obviously messed up a few parts. Sorry again, but thanks for the review! I'll get it cleaned up. |
- Change validate() to return nothing on success (matches v1.x behavior) - Extend Base.isvalid instead of exporting custom isvalid - Restore copyright headers in src/schema.jl - Remove main branch from CI.yml (only master exists) - Fix URLs: JuliaServices -> JuliaIO in README.md and docs - Fix migration.md to reflect actual v2.0 API behavior - Update tests for new validate return type Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
What is the downside to this living in JSON.jl? |
- Change validate() to return nothing on success (matches v1.x behavior) - Extend Base.isvalid instead of exporting custom isvalid - Restore copyright headers in src/schema.jl - Remove main branch from CI.yml (only master exists) - Fix URLs: JuliaServices -> JuliaIO in README.md and docs - Fix migration.md to reflect actual v2.0 API behavior - Update tests for new validate return type Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Summary
This is a complete rewrite of JSONSchema.jl that transforms it from a pure validation library to a schema generation and validation library.
New Features
schema(T): Generate JSON Schema from Julia struct definitionsSchema{T}: Type-parameterized schema for type-safe validation$refsupport for schema deduplication and recursive typesValidationResultstruct with detailed error messagesBreaking Changes
validate()now returnsValidationResultinstead ofnothing/SingleIssueJSON.schema/validate/isvalidconvenience methods removed (useJSONSchema.*directly)parent_dirkwarg removed fromSchemaconstructorBackwards Compatibility Layer
Most v1.x code will continue to work:
schema.datafield access (maps toschema.spec)Schema(true)/Schema(false)boolean schemasvalidate(data, schema)andisvalid(data, schema)inverse argument orderrequiredvalidation withoutpropertiesdiagnose()function (deprecated but functional)SingleIssuetype aliasOther Changes
Test plan
🤖 Generated with Claude Code