marl / jams

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

number type in namespace definition allows complex numbers #35

Closed justinsalamon closed 9 years ago

justinsalamon commented 9 years ago

The number type in namespace definitions accepts complex numbers. This means that this test for an invalid pitch_hz sequence will not raise an exception for all the test cases, because 1j is of type number. Admittedly this is a bit of a marginal case, but do we want to explicitly disallow complex numbers in the namespace definitions? (and can we?)

bmcfee commented 9 years ago

Huh. That's actually quite ambiguous in the json-schema specification --- see also here --- to the point where I wonder if it's actually correctly implemented in the jsonschema package.

I suspect that it's not correctly implemented, since python complex types are not (by default) JSON serializable. I don't see how it would make sense for them to pass schema validation but fail to serialize.

Maybe we should punt this upstream?

bmcfee commented 9 years ago

[Adding ref to #26 ]

Patching around this from inside jams seems like a severe headache. I recommend marking this as 'wontfix', and maybe posting an issue to jsonschema.

Meanwhile, can we drop 1j as a "should fail" numeric test case?

justinsalamon commented 9 years ago

sure thang chief: 49e869385b5a40e0f859059567bd595bd25385b0