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

Attached detached lite #390

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

Conversation

ptsefton
Copy link
Contributor

@ptsefton ptsefton commented Jan 10, 2025

This is another approach to clarifying Attached vs Detached that does not introduce much new terminology or conformsTo etc. Again, this is a first pass to see if this approach makes sense, it will need checking.

I moved a bit of stuff around in structure.md but there are no major changes there except to cover how to deal with a Detached RO-Crate Package. Also, did not try to deal with how you'd link one to a website.

In the section on Data Entites i further tidied up the logic around @ids and contentUrls -- I think this has made it clearer, and I don't think it will be hard to implement.

Thanks for your feedback @simleo about the complexity in my last try.

@ptsefton ptsefton requested review from elichad, stain and simleo and removed request for elichad January 10, 2025 04:49
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
Copy link
Contributor

@stain stain left a comment

Choose a reason for hiding this comment

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

Went through with @elichad and we have indicated some changes. I didn't complete the review.

docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
* The `@id` MUST be a relative path that resolves to a directory that is present in the _RO-Crate Root_.

For a _Detached RO-Crate Package_:
* If the `@id` is a _URI Path it MAY be used to create a directory and MAY resolve to a service which returns a list of files
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar issue as above, e.g. @id: "folder/subfolder/"from detached cratehttp://example.com/api/get_crate?1239` would wrongly resolve to http://example.com/api/folder/subfolder/

docs/_specification/1.2-DRAFT/root-data-entity.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/root-data-entity.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/root-data-entity.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/root-data-entity.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/root-data-entity.md Outdated Show resolved Hide resolved
@simleo
Copy link
Contributor

simleo commented Jan 14, 2025

I think appendix/relative-uris should also be reviewed in this PR, it has a lot of references to attached and detached crates.

@elichad
Copy link
Contributor

elichad commented Jan 27, 2025

Quick comment with my understanding of the primary spec changes in this PR (i.e. ignoring restructuring & rephrasing):

Previously

This table represents (my understanding of) the difference between attached/detached crates prior to this PR:

Data Entity @ids all absolute URIs Data Entity @ids include local URIs
RDE has @id ./ Attached Attached
RDE has @id absolute URI Detached Not permitted

The RDE @id was a major factor (by proxy) in defining whether a crate was attached or detached.

Now

Data Entity @ids all absolute URIs Data Entity @ids include local URIs
RDE has @id ./ Attached Attached
RDE has @id absolute URI Attached or Detached Attached

The RDE @id can no longer be used as a proxy for whether a crate is attached or detached. Instead this PR makes clearer the following distinction that it's about how a crate is being distributed or processed:

  • an Attached crate package is a directory containing the metadata file
  • a Detached crate package is a standalone metadata document

I'll add specific review comments tomorrow (out of time today) but @ptsefton does this sound right to you?

@ptsefton
Copy link
Contributor Author

ptsefton commented Jan 27, 2025 via email

Copy link
Contributor

@elichad elichad left a comment

Choose a reason for hiding this comment

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

Here are my nitpicks as promised :) I'm happy with the overall requirements changes as summarised in #390 (comment), so these comments focus on wording and structure.

Some outstanding to-do items that aren't covered by the comments:

  • Add localPath to RO-Crate context (via ro-terms?) [could make a separate issue]
  • Update other references to attached/detached crates in line with the changes in this PR [could make a separate issue]. In particular:
    • data-entities.md "Determining entity identifier for a referenced RO-Crate"
    • relative-uris.md appendix, particularly the sections on converting between different types of crate
  • change {:note} callouts to new style
  • cross-link referenced sections where possible
  • fix CI failures
  • final proofread after comments are resolved

docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved

_Data Entities_ can also be other types, for instance an online database. These SHOULD have a `@type` of [CreativeWork] (or one of its subtypes) and typically have a `@id` which is an absolute URI.
_Data Entities_ representing directories MUST have `Dataset` as a value for `@type`. The term _directory_ here includes HTTP file listings where `@id` is an absolute URI, however "external," _Detached_ directories SHOULD have a programmatic listing of their content (e.g. another RO-Crate). It follows that the _RO-Crate Root_ is itself a data entity.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear to me what "external" / "detached" directories means here, and how these differ from the HTTP file listings mentioned right before. I'm not an expert on this aspect!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure how to word this either - @stain can you help?

docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
* The `@id` MUST be a relative path that resolves to a directory that is present in the _RO-Crate Root_.

For a _Detached RO-Crate Package_:
* If the `@id` is a _URI Path it MAY be used to create a directory and MAY resolve to a service which returns a list of files
Copy link
Contributor

Choose a reason for hiding this comment

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

Think we can now remove "it MAY be used to create a directory" as we have moved away from the idea of @id as suggested file path

docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/structure.md Outdated Show resolved Hide resolved
docs/_specification/1.2-DRAFT/data-entities.md Outdated Show resolved Hide resolved
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.

4 participants