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

Update algorithms to use new processing model. #30

Open
wants to merge 43 commits into
base: main
Choose a base branch
from

Conversation

wes-smith
Copy link
Contributor

@wes-smith wes-smith commented Aug 27, 2024

index.html Outdated Show resolved Hide resolved
index.html Outdated
Comment on lines 601 to 604
that is needed to compress or decompress to/from CBOR-LD. During compression and decompression, the
<a href="top-level-term-codec-map-generation-algorithm"></a> is run once during setup, and the
<a href="update-term-codec-map-with-type-scoped-context-algorithm"></a> and
<a href="update-term-codec-map-with-property-scoped-context-algorithm"></a> are run on an as-needed basis.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
that is needed to compress or decompress to/from CBOR-LD. During compression and decompression, the
<a href="top-level-term-codec-map-generation-algorithm"></a> is run once during setup, and the
<a href="update-term-codec-map-with-type-scoped-context-algorithm"></a> and
<a href="update-term-codec-map-with-property-scoped-context-algorithm"></a> are run on an as-needed basis.
that is needed to compress to or decompress from CBOR-LD. For compression and decompression, the
<a href="top-level-term-codec-map-generation-algorithm"></a> is run once during setup, and the
<a href="update-term-codec-map-with-type-scoped-context-algorithm"></a> and
<a href="update-term-codec-map-with-property-scoped-context-algorithm"></a> are run on an as-needed basis.

index.html Outdated
<section>
<h2>Top-level Term Codec Map Generation Algorithm</h2>
<p>
This algorithm takes a map `typeTable` and an array of JSON-LD contexts `contexts` and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be like the other algorithms, with ({datatype} {name}) or similar for each input and output?

Suggested change
This algorithm takes a map `typeTable` and an array of JSON-LD contexts `contexts` and
This algorithm takes a map `typeTable` and an array of JSON-LD contexts `contexts` as inputs, and

index.html Outdated
<li>Let `termToEncodedValue` be an ordered map.</li>
<li>
For each value in `contextUrls`, dereference the JSON-LD contexts and process
every entry at the top level (i.e. ignoring type and property-scoped contexts).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the parenthetical meant to be read as ignoring type-scoped and property-scoped contexts? or ignoring (type) and (property-scoped contexts)? or something else? I'm guessing it's the first, which calls for the addition of this hyphen —

Suggested change
every entry at the top level (i.e. ignoring type and property-scoped contexts).
every entry at the top level (i.e., ignoring type- and property-scoped contexts).

index.html Outdated
</ol>
</li>
<li>
Sort the keys in `termToEncodedValue` by value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort order should be more explicit. I'm guessing it should be by codepoint value, which sometimes means we need to force a codepage or call for it to be declared (e.g., UTF-8, UTF-16, UTF-32, etc.)....

index.html Outdated
<li>
For each entry in the typeTables array in the CBOR-LD Varint Registry Entry associated with
`registryEntryId`, dereference the URL if necessary and add ${type}: ${table} from the resulting
document to `typeTable`. If "callerProvidedTable" appears in contextTables, populate `typeTables`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
document to `typeTable`. If "callerProvidedTable" appears in contextTables, populate `typeTables`
document to `typeTable`. If "callerProvidedTable" appears in `contextTables`, populate `typeTables`

index.html Outdated
<section>
<h2>Get CBOR-LD Registry Entry ID Algorithm</h2>
<p>
This algorithm takes a CBOR-LD payload `cborldBytes` as input and returns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This algorithm takes a CBOR-LD payload `cborldBytes` as input and returns
This algorithm takes a CBOR-LD payload `cborldBytes` as input, and returns

index.html Outdated
Comment on lines 911 to 915
If the CBOR tag on `cborldBytes` is not in the range `0x0600-0x06FF`, throw an
ERR_NON_CBOR_LD_TAG error.
</li>
<li>
Otherwise if the CBOR tag on `cborldBytes` is in the range `0x0600-0x067F`, set `registryEntryId` to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If the CBOR tag on `cborldBytes` is not in the range `0x0600-0x06FF`, throw an
ERR_NON_CBOR_LD_TAG error.
</li>
<li>
Otherwise if the CBOR tag on `cborldBytes` is in the range `0x0600-0x067F`, set `registryEntryId` to
If the CBOR tag on `cborldBytes` is not in the range `0x0600`–`0x06FF`, throw an
`ERR_NON_CBOR_LD_TAG` error.
</li>
<li>
Otherwise if the CBOR tag on `cborldBytes` is in the range `0x0600`–`0x067F`, set `registryEntryId` to

index.html Outdated
<ol>
<li>
If the last byte of the CBOR tag does not form the first byte of a valid varint, throw an
ERR_INVALID_VARINT_VALUE error.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ERR_INVALID_VARINT_VALUE error.
`ERR_INVALID_VARINT_VALUE` error.

index.html Outdated
Generate Decompressed JSON-LD Algorithm
</h2>
<p>
This algorithm takes as input CBOR-LD payload `cborldBytes` and a map `typeTable` and returns a JSON-LD document.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This algorithm takes as input CBOR-LD payload `cborldBytes` and a map `typeTable` and returns a JSON-LD document.
This algorithm takes a CBOR-LD payload `cborldBytes` and a map `typeTable` as inputs, and returns a JSON-LD document.

@wes-smith wes-smith marked this pull request as ready for review January 6, 2025 01:25
Copy link
Member

@dlongley dlongley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a significant improvement over what we have in the spec today, so we should merge this and iterate from there.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
From @TallTed.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
@dlongley dlongley requested a review from TallTed January 6, 2025 15:48
@dlongley
Copy link
Member

dlongley commented Jan 6, 2025

I believe I've applied all of @TallTed's suggestions that still remained after the most recent edits.

@wes-smith
Copy link
Contributor Author

@TallTed How do you want to proceed here? This PR is pretty huge - do you intend to do a full rereview, and if so, how long do you think it will take?

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial. Almost only commas and code fences, for clarity.

Set `state.strategy` to "decompression".
</li>
<li>
For each entry `type`: `map` in the `typeTables` array in the CBOR-LD Varint Registry Entry
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be something that sets `type`: `map` aside from the rest of the text. <em> might be enough. I'm open to better ideas.

Suggested change
For each entry `type`: `map` in the `typeTables` array in the CBOR-LD Varint Registry Entry
For each entry <em>`type`: `map`</em> in the `typeTables` array in the CBOR-LD Varint Registry Entry

<h3>Convert Document Algorithm</h3>
<p>
This algorithm takes a map `state` and a map or array of
maps `inputDocuments` and returns a map containing a map `state` and a map or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
maps `inputDocuments` and returns a map containing a map `state` and a map or
maps `inputDocuments`, and returns a map containing a map `state` and a map or

<section>
<h4>Convert Value for Compression Algorithm</h4>
<p>
This algorithm takes maps `state` and `termInfo` and values `valueToEncode` and `termType`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This algorithm takes maps `state` and `termInfo` and values `valueToEncode` and `termType`
This algorithm takes maps `state` and `termInfo`, and values `valueToEncode` and `termType`,

<section>
<h4>Get Input Entries for Compression Algorithm</h4>
<p>
This algorithm takes maps `state`, `activeContext`, and `input` and returns a map `state` and an array `entries`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This algorithm takes maps `state`, `activeContext`, and `input` and returns a map `state` and an array `entries`.
This algorithm takes maps `state`, `activeContext`, and `input`, and returns a map `state` and an array `entries`.

<section>
<h4>Get Object Types for Compression Algorithm</h4>
<p>
This algorithm takes maps `activeContext` and `input` and returns a set
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This algorithm takes maps `activeContext` and `input` and returns a set
This algorithm takes maps `activeContext` and `input`, and returns a set

Set `varintArray` to an array containing the varint representation of `registryEntryId`.
</li>
<li>
Set `varintTagValue` to `varintArray[0]` appended to the end of the bytes 0xD906.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Set `varintTagValue` to `varintArray[0]` appended to the end of the bytes 0xD906.
Set `varintTagValue` to `varintArray[0]` appended to the end of the bytes `0xD906`.

</li>
<li>
Set `varintBytesValue` to a CBOR byte string containing the rest of `varintArray` appended
to the end of the bytes 0x82.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
to the end of the bytes 0x82.
to the end of the bytes `0x82`.

</ol>
</li>
<li>
Otherwise if `valueToDecode` is an integer and `state`.`typesEncodedAsBytes` does not
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Otherwise if `valueToDecode` is an integer and `state`.`typesEncodedAsBytes` does not
Otherwise, if `valueToDecode` is an integer and `state`.`typesEncodedAsBytes` does not

</ol>
</li>
<li>
Otherwise if `valueToDecode` is a byte array and `tableType` is not "none", set
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Otherwise if `valueToDecode` is a byte array and `tableType` is not "none", set
Otherwise, if `valueToDecode` is a byte array and `tableType` is not "none", set

ERR_NON_CBOR_LD_TAG error.
</li>
<li>
Otherwise if the CBOR tag on `cborldBytes` is in the range `0x0600`-`0x067F`, set `registryEntryId` to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Otherwise if the CBOR tag on `cborldBytes` is in the range `0x0600`-`0x067F`, set `registryEntryId` to
Otherwise, if the CBOR tag on `cborldBytes` is in the range `0x0600`-`0x067F`, set `registryEntryId` to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants