Conversation
4d36d6e to
79972eb
Compare
|
HiGlass validates with ajv in the front end. Needing to change the test to accommodate the new schema version isn't ideal, but I guess we really aren't using this schema anywhere (since higlass still uses the manually edited |
manzt
left a comment
There was a problem hiding this comment.
Looks good to me—left a couple of comments. There wasn’t much context in the issue or PR, so I just want to confirm: is this ready for final review, or are there more changes coming? Also, was this migration prompted by any specific issues with Pydantic v1, or is it just to stay up to date?
Obviously this introduces breaking changes for higlass-python, so any context would be helpful.
src/higlass_schema/schema.py
Outdated
| ("$schema", "http://json-schema.org/draft-07/schema#"), | ||
| ("$id", "https://higlass.io/#viewconf"), | ||
| ("$schema", GenerateJsonSchema.schema_dialect), | ||
| ("$id", "https://higlass.io/schemas/viewconf"), |
There was a problem hiding this comment.
The "$id" field in JSON Schema is used to define a unique identifier (URI) for the schema. I copied the previous one from the original schema, but apparently it's not a real link.
Is the URL you specified supposed to be a real link to the schema? Otherwise I say we remove it.
|
Sorry for the lack of context! The migration was prompted by dependency conflicts between higlass-python and other packages in my environments since other packages have migrated to V2. It's ready for final review. |
|
Sounds good, I've made some changes on a separate branch to align the JSON schema generation because it's quite a bit different with this PR. I'll merge and open a separate PR. |
Resolves #15