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

JSONSchema compliance for namespaces #213

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MCMcCallum
Copy link

@MCMcCallum MCMcCallum commented Jan 2, 2021

  • Addresses issue Next generation jams #208 phase 1
  • Updated JAMs schema to be json schenma compliant
  • Previously dense observation types are now entered as a single observation in the Annotation observation list, they no longer redefine the observation list itself, rather, just the elements within it. This means that all Annotations now have a list of observations, the elements of which can be dense observations themselves
  • the import_labs function has been moved from the util module to the core module. In a future PR it should be modified to be a constructor for JObjects
  • Fixed serialization JAMs for bools, lists and dicts
  • Updated jsonschema to latest version - necessary for determining namespaces based on a const namespace property
  • Local namespaces are no longer supported - a necessity to become jsonschema compliant
  • Version bumped
  • Tests updated to adhere to new schema for objects with DenseObservations
  • Removed spurious validation and schema methods / attributes in jams.core module
  • add_namespace and is_dense is removed from the jams.schema module. All namespaces are now available in the jams.schema.NAMESPACES dictionary
  • schema for each observation type is now available in the jams.schema.OBSERVATIONS_SCHEMA dictionary

 - Previously dense observation types are now entered as a single observation in the Annotation observation list, they no longer redefine the observation list itself, rather, just the elements within it. This means that all Annotations now have a list of observations, the elements of which can be dense observations themselves
 - the import_labs function has been moved from the util module to the core module. In a future PR it should be modified to be a constructor for JObjects
 - Fixed serialization JAMs for bools, lists and dicts
 - Updated jsonschema to latest version - necessary for determining namespaces based on a const namespace property
 - Local namespaces are no longer supported - a necessity to become jsonschema compliant
 - Version bumped
 - Tests updated to adhere to new schema for objects with DenseObservations
 - Removed spurious validation and __schema__ methods / attributes in jams.core module
 - add_namespace and is_dense is removed from the jams.schema module. All namespaces are now available in the jams.schema.NAMESPACES dictionary
 - schema for each observation type is now available in the jams.schema.OBSERVATIONS_SCHEMA dictionary
@justinsalamon
Copy link
Contributor

Hey! Is this in response to an issue? Was there a side convo I’m missing?

@justinsalamon
Copy link
Contributor

Ah, #208

this seems like a pretty substantial design change 😬 we typically try to hash those out in issues before implementing 😅

I believe @bmcfee has some fairly concrete thoughts on dense vs sparse annotation types.

My primary concern is ensuring major changes don’t break Scaper.

Let’s discuss!

@bmcfee bmcfee added interoperability Making JAMS play nice with other packages schema Issues pertaining to schema definitions labels Mar 10, 2021
@bmcfee
Copy link
Contributor

bmcfee commented Mar 10, 2021

this seems like a pretty substantial design change grimacing we typically try to hash those out in issues before implementing sweat_smile

@MCMcCallum and I had actually hashed this out quite a bit before the implementation, but it was out-of-band.

Sorry that I haven't had time to dig into this PR thoroughly btw. Teaching in a pandemic, you know 😬

My primary concern is ensuring major changes don’t break Scaper.

The plan in #208 , phase 1, is to retain backward compatibility to the previous schema. However, dynamic namespace support will eventually dropped, and scaper will likely need to adapt in response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interoperability Making JAMS play nice with other packages schema Issues pertaining to schema definitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants