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

Declare that order is unimportant in all "ordered sets" #656

Closed
TallTed opened this issue Feb 16, 2021 · 7 comments
Closed

Declare that order is unimportant in all "ordered sets" #656

TallTed opened this issue Feb 16, 2021 · 7 comments
Assignees
Labels
editorial Editors should update the spec then close pending close Issue will be closed shortly if no objections

Comments

@TallTed
Copy link
Member

TallTed commented Feb 16, 2021

I do not think the single note <p class="note" title="Ordering of values"> about this is sufficient.

I think adding such an inline declaration every time is important, because the INFRA doc to which every instance of "ordered set" links includes notes of --

Almost all cases on the web platform require an ordered set, instead of an unordered one, since interoperability requires that any developer-exposed enumeration of the set’s contents be consistent between browsers. In those cases where order is not important, we still use ordered sets; implementations can optimize based on the fact that the order is not observable.

-- and --

As with ordered sets, by default we assume that maps need to be ordered for interoperability among implementations.

-- which both loudly hint that order is important for interop.

Could be as simple as adding (although the order of elements is unimportant within DID Core, DID methods are free to specify an ordering algorithm that must be used for interop within that DID method) or similar to all property MUST be ... an <a data-cite="INFRA#ordered-set">ordered set</a> ... statements.

This would not be necessary if all the current links to "INFRA#ordered-set" instead went to our own discussion thereof, with the "order doesn't matter" declaration, and then linked out to the INFRA spec.

@iherman
Copy link
Member

iherman commented Feb 16, 2021

This would not be necessary if all the current links to "INFRA#ordered-set" instead went to our own discussion thereof, with the "order doesn't matter" declaration, and then linked out to the INFRA spec.

I would certainly prefer this than adding that extra note everywhere in the document (which would make reading fairly difficult).

I am not convinced it is necessary, though.

@msporny
Copy link
Member

msporny commented Feb 16, 2021

which both loudly hint that order is important for interop.

Hmm, depends on the sort of interop you're talking about... and that text is really speaking to "code running on different browsers when iterating over an Object over the years have tried to make sure they preserve insertion order"... which is very different than "ordering of the information model" (e.g., RDF Dataset Canonicalization).

I don't think we need to loudly declare this in the specification more than once. We should loudly declare it once, though. We're just saying -- you can't depend on order, so unless you have some ordering mechanism that you're sure works (like RDF Dataset Normalization)... don't assume that order is preserved.

@OR13
Copy link
Contributor

OR13 commented Feb 17, 2021

#665

Changes:

Screen Shot 2021-02-17 at 10 25 17 AM

To:

Screen Shot 2021-02-17 at 10 24 35 AM

@TallTed
Copy link
Member Author

TallTed commented Feb 17, 2021

I want to make a small to the text within that box ... but it's further away from the single-word change you made than github will allow me to make a suggestion on.

I'd change --

    <p class="note" title="Ordering of values">
As a result of the <a href="#data-model">data model</a> being defined using
terminology from [[INFRA]], property values which can contain more than one
item, such as <a data-cite="INFRA#ordered-map">maps</a> and
<a data-cite="INFRA#ordered-set">sets</a>, are explicitly ordered. For the
purposes of this specification, unless otherwise stated, ordering is not
important and implementations are not expected to produce or consume
deterministically ordered values.
    </p>

-- to --

    <p class="advisement" title="Ordering of values">
As a result of the <a href="#data-model">data model</a> being defined using
terminology from [[INFRA]], property values which can contain more than one
item, such as <a data-cite="INFRA#ordered-map">maps</a> and
<a data-cite="INFRA#ordered-set">sets</a>, are explicitly ordered. (All
list-like value structures in [[INFRA]] are ordered, whether or not that 
order is significant.) For the
purposes of this specification, unless otherwise stated, ordering is not
important and implementations are not expected to produce or consume
deterministically ordered values.
    </p>

To be clear, I've added (All list-like value structures in [[INFRA]] are ordered, whether or not that order is significant.)

And I still think that all the links to "INFRA#ordered-set" within the document should be changed to an internal description that includes something like "The order of values in this list-like structure isn't significant, even though INFRA says it is" before the reader gets to the INFRA site.

@TallTed
Copy link
Member Author

TallTed commented Feb 19, 2021

@msporny noted in #665 --

in reply to @TallTed:

And I still think that all the links to "INFRA#ordered-set" within the document should be changed to an internal description that includes something like "The order of values in this list-like structure isn't significant, even though INFRA says it is" before the reader gets to the INFRA site.

Will contemplate how to do this cleanly... you may want to raise an issue for it so we don't forget, @TallTed.

This (#656) remains that issue.

@msporny
Copy link
Member

msporny commented Feb 26, 2021

To be clear, I've added (All list-like value structures in [[INFRA]] are ordered, whether or not that order is significant.)

This was added to the spec a bit ago... forget which PR, but it's in there now along with more changes, to drive your point home @TallTed, in 218f6ff. I also changed "ordered set" -> "set" to not convey that order is important for sets and cross-checked the entire specification to make sure we don't depend on set ordering anywhere (we don't).

I'm hesitant to belabor the point more in the spec. I'm marking this pending close, folks that don't feel like we have closure on this yet, feel free to object to closing.

@msporny msporny added editorial Editors should update the spec then close pending close Issue will be closed shortly if no objections and removed pre-cr-p3 labels Feb 26, 2021
@TallTed
Copy link
Member Author

TallTed commented Feb 26, 2021

I'll hope the sum of changes to date is sufficient. We'll find out!

I'm OK with closing this one.

@msporny msporny closed this as completed Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Editors should update the spec then close pending close Issue will be closed shortly if no objections
Projects
None yet
Development

No branches or pull requests

4 participants