ga4gh-beacon / beacon-v2-Models

Models that leverage the Beacon Framework v2
Apache License 2.0
4 stars 7 forks source link

aachange -> aminoacidSubstitution #25

Closed mbaudis closed 2 years ago

mbaudis commented 3 years ago

The aachange parameter name is odd and not in line with general attempts to stick w/ readability... We have longer names already :-) (pathologicalTnmFinding, sampleOriginDetail ...) so let's just use aminoacidSubstitution.

https://github.com/ga4gh-beacon/beacon-v2-Models/blob/5136123994e19c505ef1c6360369c38b8fcc3af9/BEACON-V2-draft4-Model/genomicVariations/requestParameters.json#L64

mbaudis commented 3 years ago

Also: aminoacidChanges is being used elsewhere:

    "MolecularAttributes": {
      "geneIds": {
        "type": "array",
        "items": "string"
      },
...
      "aminoacidChanges": {
        "description": "Lisf of change(s) at aminoacid level for protein affecting variants.",
        "type": "array",
        "items": {
          "type": "string"
        },
        "example": ["V304*"]
      }
    },
jrambla commented 3 years ago

@mbaudis the style used should be (except errors or omissions):

mbaudis commented 3 years ago

@jrambla But where is this defined? And the query string length argument (while appreciated from a "see what you do in the browser bar while testing" doesn't really hold much, IMO, with the API going full POST (with some GET support tacked on?).

Anyway, clarity & consistency >> saving characters. Also https://github.com/ga4gh-beacon/beacon-v2-Models/issues/28#issuecomment-896572954

jrambla commented 3 years ago

@mbaudis In the call this week David discussed about the difference between aminoacidSubstitution and aminoacidChange. "substitution" pointing to switching one aa for another, while "change" is much more open to any type of alteration. If you agree with David in the distinction, then, I believe the latter is a better fit for the concept we are aiming to represent in this property. Let me know and we'll proceed with updating the schema accordingly.