search

ga4gh / vrs

Extensible specification for representing and uniquely identifying biological sequence variation
https://vrs.ga4gh.org
Apache License 2.0
80 stars 34 forks source link

Change IndefiniteRange.value to be an integer #498

Closed theferrit32 closed 3 months ago

theferrit32 commented 5 months ago

See https://github.com/ga4gh/va-spec/issues/129

larrybabb commented 5 months ago

@theferrit32 can you shed more light on why this was needed?

theferrit32 commented 5 months ago

Having this field (IndefiniteRange.value) be an integer to match the type of the Number.value field makes the graphQL query for those objects a little nicer because you can just ask for a field called value of type integer and it works for both IndefiniteRange and for Number

See item (1) here: https://github.com/broadinstitute/gnomad-browser/issues/1472#issue-2223648155

Here in the graphQL query if I want both IndefiniteRanges and Numbers to be returned in the same response I have to alias one of those value fields to have a different name, since the types don't match: https://gist.github.com/theferrit32/df380899e9974cb885c2780cfa82ce7a#file-gnomad_vrs_va-graphql-L9

larrybabb commented 4 months ago

@ahwagner I will defer to you on this one. If we merge this I think we may need a 1.4 release (unless you think it is acceptable to backport this into 1.3?). Please provide your perspective and proceed with whichever path you see as proper.

dazzariti commented 4 months ago

@larrybabb will review, remove Andreas as reviewer

ahwagner commented 3 months ago

It seems to me like this would be best as a patch version. In JSON Schema, an integer is just a constrained JSON number. The use of a non-integer number does not conform to the information content we expect, so this shouldn't be breaking in any way, just clarifying, and therefore appropriate for patch level.