-
Notifications
You must be signed in to change notification settings - Fork 169
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
[ENH] Clearly define "entity" in common principles #947
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am generally in favor of this addition.
But I think it would be worth to link to the appendix:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this proposal, I am also in favor, and I agree with Remi that links to the entity pages should be supplied.
If the definition of "entity" is presented unambiguously in the specification, then it is worth considering the prospect of enforcing this nomenclature throughout the specification
I think replacing mentions of "key value pairs" with entity (and back referencing to the definition) is a good idea.
Just a thought, but I think it's probably worth it to have a schema file for the common principles, which could then be rendered into a list with a macro. |
f43f036
to
00f9cea
Compare
Done (00f9cea); but feel free to propose alternative phrasing.
I can give that a go, but I might try it in a separate branch based off of this one and generate a dedicated PR for that proposal.
Were you thinking of such for specifically the definitions within the common principles? Regardless, might be better documented as a standalone Issue so it doesn't get lost in here. |
For anyone interested in the prospect of adopting the "entity" term more broadly across the specification, please see Lestropie#1. |
Actually read this too quickly and now I have an existential question. Is an I had pretty much understood Seems that defining |
I understood entity as the key value pair :-D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, even better together with Lestropie#1
although as you mention in #950, the
allowed, OPTIONAL or REQUIRED
should maybe be amended
Yeah, crossed my mind too. Given it is used strictly in reference to file names, where keys cannot appear in the absence of a value, I think that from a utilitarian perspective having the term refer to the pair is preferable. It does make things slightly muddy when e.g. referring to appendices that relate to entities; but that documentation doesn't only specify a list of keys, they also specify for each the requirements for the corresponding value in order to form a valid entry in the file name. So one can kinda still point to a "list of entities", even if the list is indexed by "entity keys", as the expectations for the entity values in order to form a valid entity is still there in the list content. Is a GitHub poll needed? :-P |
maybe we first ask @bids-standard/maintainers who have not seen this, and then decide whether we need to discuss this more widely. |
[FIX] Standardise use of "entity" definition across specification
I think the only thing that feels weird by defining the Either way I am sure I can adjust my mental model but some clarification would be good. |
Sorry, I have always been confused about entities, and I am more confused after the discussion. The section called Entity table might be helped by some clarification. Are As far as I can tell |
FWIW, I've always considered the entity to be the key only (e.g., "run entity"), with the combination of entity and label/index being the "key-value pair" we refer to in the specification, but I'm not heavily invested in that distinction. I will note that I think folks say "the run entity" more often than they say "the run entity key" or "the run-index entity", though.
@Remi-Gau I mostly took descriptions from the specification, but I can't guarantee that my own interpretation didn't bleed into any changes I made in the schema and in the appendices. I think saying "entity keys" in the Entities appendix would make things clearer in any case.
@VisLab,
Could you expand on this? Some modalities/suffixes are sort of "auxiliary" in that they are associated with some other file containing imaging data, such as events files or physio files associated with a BOLD scan, but they can also exist on their own. In the case of events files, if you wanted to have those files in the absence of imaging files, you would have to have them in the |
The Entity table lists the things that I thought of as "modalities" (or "suffixes") in the Entity column. I think that re-titling and re-captioning this table as well as the appendix itself could improve clarity. @tsalo: This was a useful observation:
|
There's actually three options here, not two. Which of the following is an example of an entity?
From subsequent text in "Common principles" - "File name structure":
This implies option 1, which has been thus far omitted from the discussion... |
Note: Option 1 seems to be what the title of the Entity Table in the appendix is using. (Though I think that most people reading the table would think that the Entity title refers to items in column 1 and not to the meaning of the other titles in the header.) |
One other comment.... when I think of "entity", I can see "subject" and "task" being entities. I find it hard to think of a key or a key-value pair having some grounding in reality, which I think the word "entity" implies. This may be part of the source of the confusion. |
at a generic level, one could borrow the description of entity from the prov ontology: https://www.w3.org/TR/prov-dm/#term-entity . as we are working on a prov extension for bids, aligning with such a description, and potentially augmenting it with bids/neuroimaging examples, may also lay some semantic groundwork for the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. Minor suggestion.
RE @satra's suggestion that we derive from "An entity is a physical, digital, conceptual, or other kind of thing with some fixed aspects; entities may be real or imaginary." I don't see how that fits with our usage, which is closer to the concept of "morpheme", IMO.
src/02-common-principles.md
Outdated
@@ -105,6 +105,12 @@ misunderstanding we clarify them here. | |||
The modality may overlap with, but should not be confused with | |||
the **data type**. | |||
|
|||
1. **Entity** - a portion of a file name, consisting of a **key** and corresponding | |||
**value** separated by a hyphen. Supported entities are defined in | |||
[Appendix IX](99-appendices/09-entities.md); further, whether they are allowed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a case that is allowed but neither OPTIONAL or REQUIRED, so we should avoid introducing a term that implies a distinction.
[Appendix IX](99-appendices/09-entities.md); further, whether they are allowed, | |
[Appendix IX](99-appendices/09-entities.md); further, whether they are |
And the existential descent continues... 😬 "Morpheme" is rather obscure and doesn't convey the 2-tuple nature. "Attribute" may be closer? Would however require fairly extensive changes to substitute such a core terminology entirely. |
To me key-value pairs are entity descriptors, entity designators, or entity
specifiers not entities.
…On Tue, Dec 14, 2021 at 4:11 PM Robert Smith ***@***.***> wrote:
And the existential descent continues... 😬
"Morpheme" is rather obscure and doesn't convey the 2-tuple nature. "
Attribute <https://en.wikipedia.org/wiki/Attribute_(computing)>" may be
closer? Would however require fairly extensive changes to substitute such a
core terminology entirely.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#947 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJCJOTGG64EZZC5UFGNR23UQ66JLANCNFSM5JNLZKWA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Following from my prior comment:
I'm happy to have a go at redressing this myself, but it's going to be relatively laborious to adopt a common definition across the entire specification, so I don't really want to be generating hypothetical PRs for all of the different alternatives. Let's see if this works... Edit: I think you just click on your preferred option and the poll result updates, but I've never used it before... |
Bumping seeking maintainer feedback on alternative proposal in comment above; I'm unable to progress on this proposal based on poll results. |
I like the proposal from #947 (comment). Let's collect some more opinions cc @bids-standard/maintainers @hoechenberger @adam2392 |
Invested / interested parties please see Lestropie#4, which is intended as an augmentation of this PR based on prior discussion here. |
Common principles: More elaborate "entity" definition
Resolves bids-standard#947 against: - bids-standard#962 (21b7725) - bids-standard#1044 (d9eeb6d) - bids-standard#867 (5fe26e1) - bids-standard#998 (e47283a) Conflicts: src/02-common-principles.md src/schema/README.md src/schema/objects/entities.yaml src/schema/objects/metadata.yaml
Codecov Report
@@ Coverage Diff @@
## master #947 +/- ##
==========================================
+ Coverage 70.75% 71.71% +0.96%
==========================================
Files 9 9
Lines 937 937
==========================================
+ Hits 663 672 +9
+ Misses 274 265 -9
Continue to review full report at Codecov.
|
Lestropie#4 has been merged, and 699b74e brings this up-to-date with master (see commit message for conflicts). PR is no longer flagged as draft. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Thanks @Lestropie and sorry for the delay with reviews and merging. |
Just realised why the "entity" terminology when referring to key-values in file names hadn't clicked for me: it's missing from the list of definitions!
While this is a relatively trivial addition, I am nevertheless listing as a draft PR. I have noticed that in subsequent text, both "entity / entities" and "key-value(s) / key-value pairs" are used interchangeably. If the definition of "entity" is presented unambiguously in the specification, then it is worth considering the prospect of enforcing this nomenclature throughout the specification, and I think this is an appropriate location for that discussion.