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

[FIX] Standardise use of "entity" definition across specification #1

Merged
merged 1 commit into from
Dec 12, 2021

Conversation

Lestropie
Copy link
Owner

Relates to discussion in bids-standard#947.

Consistently using the term "entity" to refer to key-value pairs within filenames provides a disambiguation from key-values that are stored in sidecar metadata files.

While I can definitely see a potential benefit in providing hyperlinks to the "Common principles - Definitions" entries whenever these defined terms are used in the spec, that's likely a much larger changeset, so may be deferred to a separate Issue at bids-standard/bids-specification if there is interest.

@Lestropie Lestropie self-assigned this Dec 8, 2021
key/value pair corresponds to modality suffix,
such as T1w or inplaneT1, referenced by the defacemask image.
entity corresponds to modality suffix,
such as `T1w` or `inplaneT1`, referenced by the defacemask image.
Copy link
Owner Author

Choose a reason for hiding this comment

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

Note: unrelated formatting change

@Lestropie
Copy link
Owner Author

There were uses of "key-value-pairs", "key-value", "key:value", "key/value", "keyword-value", and "parameter" all being used to refer to entities.

Copy link

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

this is great, thanks a lot @Lestropie - would you mind opening a PR to bids-specification with this?

While I can definitely see a potential benefit in providing hyperlinks to the "Common principles - Definitions" entries whenever these defined terms are used in the spec, that's likely a much larger changeset, so may be deferred to a separate Issue at bids-standard/bids-specification if there is interest.

agreed!

@Lestropie
Copy link
Owner Author

would you mind opening a PR to bids-specification with this?

Plan was to merge this first, and it would then be integrated into bids-standard#947, as this PR targets the base branch of that PR. I can't currently propose this change as a standalone PR on bids-specification, as there is no suitable base branch on the upstream repo for construction of such.

Are you happy for this to be integrated into bids-standard#947, or would you prefer to go through the gymnastics of having essentially this same proposal as a standalone over on bids-standard/bids-specification?

@sappelhoff
Copy link

Are you happy for this to be integrated into bids-standard#947, or would you prefer to go through the gymnastics of having essentially this same proposal as a standalone over on bids-standard/bids-specification?

no, that seems fine to me. No additional gymnastics needed.

@Lestropie Lestropie marked this pull request as ready for review December 12, 2021 22:51
@Lestropie Lestropie merged commit 6b81d67 into define_entity Dec 12, 2021
@Lestropie Lestropie deleted the replace_keyvalue_entity branch December 12, 2021 22:52
Lestropie pushed a commit that referenced this pull request Apr 26, 2024
ENH: Remove Atlas metadata, update imaging derivatives text around seg-
effigies pushed a commit that referenced this pull request Aug 29, 2024
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.

2 participants