marl / jams

A JSON Annotated Music Specification for Reproducible MIR Research
ISC License
186 stars 27 forks source link

validation fails hard with jsonschema 2.5.1 #51

Closed bmcfee closed 9 years ago

bmcfee commented 9 years ago

Title says it all. This doesn't show up in travis because conda still has jsonschema 2.4.0.

Running the test suite locally, I get the following:

======================================================================
ERROR: jams_test.test_jams_validate_good
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/bmcfee/git/jams/jams/tests/jams_test.py", line 539, in test_jams_validate_good
    j1.validate()
  File "/home/bmcfee/git/jams/jams/core.py", line 1187, in validate
    valid = super(JAMS, self).validate(strict=strict)
  File "/home/bmcfee/git/jams/jams/core.py", line 461, in validate
    jsonschema.validate(self.__json__, schema.JAMS_SCHEMA)
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 478, in validate
    cls(schema, *args, **kwargs).validate(instance)
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 122, in validate
    for error in self.iter_errors(*args, **kwargs):
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 98, in iter_errors
    for error in errors:
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/_validators.py", line 291, in properties_draft4
    schema_path=property,
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 114, in descend
    for error in self.iter_errors(instance, schema):
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 98, in iter_errors
    for error in errors:
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/_validators.py", line 42, in items
    for error in validator.descend(item, items, path=index):
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 114, in descend
    for error in self.iter_errors(instance, schema):
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 98, in iter_errors
    for error in errors:
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/_validators.py", line 199, in ref
    scope, resolved = validator.resolver.resolve(ref)
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 336, in resolve
    return url, self._remote_cache(url)
  File "/usr/local/lib/python2.7/dist-packages/functools32/functools32.py", line 400, in wrapper
    result = user_function(*args, **kwds)
  File "/usr/local/lib/python2.7/dist-packages/jsonschema/validators.py", line 346, in resolve_from_url
    raise RefResolutionError(exc)
RefResolutionError: unknown url type: 
bmcfee commented 9 years ago

Note that validation fails only at the top level. Validating individual annotations still works fine.

ejhumphrey commented 9 years ago

FWIW, I've run into this elsewhere, and I'm not totally convinced this isn't jsonschema's issue (i.e. not our fault). I think, with their recent upgrade, something got hosed traversing referentially defined schema. I'd check that project and see if someone else hasn't hit the same snag.

bmcfee commented 9 years ago

It's for sure due to an API change in jsonschema from 2.4 to 2.5. We just need to catch up.

ejhumphrey commented 9 years ago

That's what I'm saying, I'm not convinced it's a "catch up"?

bmcfee commented 9 years ago

Killing the "id" field in jams_schema.json actually fixes this.

EDIT: this "fix" also works in 2.4.0.

EDIT EDIT: changing "id": "#root" to "id": "JAMS" also works.

ejhumphrey commented 9 years ago

wat. which one, the top-level "id": "#root"?

bmcfee commented 9 years ago

wat. which one, the top-level "id": "#root"?

Yes. It's trying to resolve "root" against the draft-04 schema, and can't find it. Hence fail.

See also.

bmcfee commented 9 years ago

.. after reading the above article, I think the right thing to do here is kill the id field. It doesn't seem necessary here, right?

ejhumphrey commented 9 years ago

without knowing more things, I'd say that sounds reasonable.

bmcfee commented 9 years ago

Ok. Shall I close this one out then?

bmcfee commented 9 years ago

Addendum to yesterday: I think we were previously just mis-using the id field.

The id field would be useful down the line if we decide to refactor the namespace schema implementation via inheritance / schema references (see here). It does seem like jams.schema is currently doing a bit of work that probably is redundant with jsonschema.

I'm not sure yet if rewriting the namespace schema definitions to refer back to the master jams schema would allow us to simplify the implementation, but it's worth investigating (for 0.3.0).