search

ga4gh / refget

GA4GH Refget specifications docs
https://ga4gh.github.io/refget
14 stars 7 forks source link

Add draft ADR for no unordered arrays #19

Closed nsheff closed 9 months ago

nsheff commented 3 years ago

Explains rationale for decision to follow "option A" for how to encode order in the digests.

nsheff commented 3 years ago

Decision on this is pending today's discussion on the compatibility API

sveinugu commented 3 years ago

I believe I still have some significant concerns that are not solved by the committed decision, and which would at least require another specification of unordered digests on top of the current one (it's not only an implementation detail): https://github.com/ga4gh/seqcol-spec/issues/5#issuecomment-934039875. (I recently edited this comment for clarity.)

sveinugu commented 3 years ago

I have now formulated what I believe is a significant argument against the committed decision based on the observation that unordered seqcol digests logically depends on a row-wise data structure: https://github.com/ga4gh/seqcol-spec/issues/5#issuecomment-949554546

By binding seqcol to a column-wise structure, I believe one will not in practice be able to make use of the digests as identifiers for coordinate systems, which I believe was really the driving force for moving towards a column-wise structure in the first place. I feel confident that by not solving the issue of ordered and unordered seqcol digests in the specification itself, we are missing an opportunity. It may be that the solution can be built on top the current proposal, but I would like to see this thought out before I let this go. Another possibility is to support both column-wise and row-wise data structures in the spec, something I will provide a suggestion for.

nsheff commented 3 years ago

OK. As I mentioned in your comment, I think an important question is: why isn't this solved by the comparison function? I do not think moving from column-wise structure is an option, because that would sacrifice lots of other desirable properties (such as recursion).

sveinugu commented 3 years ago

OK. As I mentioned in your comment, I think an important question is: why isn't this solved by the comparison function? I do not think moving from column-wise structure is an option, because that would sacrifice lots of other desirable properties (such as recursion).

I have now provided a suggestion for a solution that will technically allow both column-wise and row-wise structures, as well as ordered and unordered digests, in the same standard, and where we can possibly leave the row-wise, ordered part for a later update of the standard or for others to specify: https://github.com/ga4gh/seqcol-spec/issues/5#issuecomment-949625650

nsheff commented 9 months ago

@sveinugu Can you approve this now? This was an old ADR that you were a bit unsure about in 2021, but I think we've moved on and solved these concerns, so this decision appears accurate at this point to me.

nsheff commented 9 months ago

I'm going to go ahead and merge this now, since it's time to clear these out -- but if you think there's a remaining issue, please feel free to edit or open a new PR with corrections.

sveinugu commented 9 months ago

Looks good to me!