-
Notifications
You must be signed in to change notification settings - Fork 57
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
add identifierNAME to NXobject #1486
base: main
Are you sure you want to change the base?
Conversation
@@ -30,5 +30,25 @@ | |||
This is the base object of NeXus | |||
</doc> | |||
<!--attribute name="name"><doc>name of instance</doc></attribute--> | |||
<field name="identifierNAME" type="NX_CHAR" nameType="partial"> |
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.
This looks like it fits in the list of reserved prefixes. Add it to that list.
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.
The other prefixes are only defined in the documentation. Should we make them available to XML by adding them here, similar to "identifierNAME"
? Or do this in a separate issue/branch/PR?
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 added identifier
to the reserved prefixes.
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.
The other prefixes are only defined in the documentation. Should we make them available to XML by adding them here, similar to
"identifierNAME"
? Or do this in a separate issue/branch/PR?
I think it is good to add them to NXobject
directly. That still doesn't automatically make them reserved (it is still technically possible to overwrite them somewhere else), but at least the docs can be clearly defined and then inherited.
I suggest to do it in a different PR since for the identifier change we already have a NIAC vote (not sure if the other changes would need a separate one).
I thought that, in the end, we decided against using "identiferNAME", instead of an identifier attribute, just called "identifier". Obviously, @sanbrock has the official record. |
If it is an attribute, then it would be added to the |
Isn't it possible also to add attributes to the NXobject class at least for groups? |
It gets messy there. Can be very precise in the schema.
…On Fri, Oct 4, 2024, 9:49 AM Ray Osborn ***@***.***> wrote:
Isn't it possible also to add attributes to the NXobject class at least
for groups?
—
Reply to this email directly, view it on GitHub
<#1486 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARMUMETIDXGDZ57Y7ZJV3TZZ2TH5AVCNFSM6AAAAABPCXF2O2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJTHA4DQMBRHE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
I added some of the feedback provided by @paulmillar in #1416 with respect to the type of identifier. He made a strong point that Note that here I am still adding
My understanding was that it shall be possible to have more than one identifier (e.g., from different services) for one object. I think this is also what @sanbrock noted. Even if identifier will become an attribute, I would suggest to use |
We have been reducing use of Line 47 in 67d8519
Please pick a noun which is not |
At the NIAC, I recall that people (including, I believe, @phyy-nx) thought the "is_persistent" tag unnecessary. We need @sanbrock to confirm, but I also believe that the attributes would just be "identifier" and "service." There is no need to add partial names here, which I agree become very messy. I have no idea whether we specifically voted on any of these issues. |
We have not voted on it but agreed that a workgrouo shall conclud with a reasonable solution in a form of a PR which can then be voted on. |
Indeed, we qgreef to use type instead of service. |
There was no real conclusion on is_persistant. Indeed, its necessity was debated, although I argued that it is acually good to know if an Idebtifier is a PID or not. |
@sanbrock, do you believe we came to any conclusion about the need for partial names? Attribute names such as "identifierDOI" are pretty ugly IMHO. If there is going to be a working group, perhaps this PR should be, at least temporarily, withdrawn. |
Note that we did agree that our solution shall allow attaching multiple identifiers (e.g. orcid, linkdin to a USER).
|
This is actually a draft PR. And we are the workgroup. Note that we wanted to invite @paulmillar too to this duscussion. |
Note that another alternative is |
NIAC suggestions on the revwrite of NXidentifirler were recorded in #1451 |
These have been also voted for in Session J. See https://www.nexusformat.org/content/NIAC2024_minutes/ |
base_classes/NXobject.nxdl.xml
Outdated
|
||
This refers specifically to an ID in the Handle system operated by the Corporation for National Research Initiatives (CNRI). | ||
|
||
Syntax: hdl:prefix/identifier |
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.
Do we need the hdl:
prefix in the syntax? I don't think this adds anything useful because the type
attribute already indicates the identifier is a Handle.
I suggest the syntax is updated to:
Syntax: prefix/identifier
Example: 123456789/abc123
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.
agreed, changed
base_classes/NXobject.nxdl.xml
Outdated
|
||
The IGSN is a unique identifier assigned to a specific sample or specimen in the context of scientific research. | ||
|
||
Syntax: https://igsn.org/{IGSN-ID} |
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'm not convinced this syntax is correct.
Per the Wikipedia article, IGSNs are now issued by DataCite.
Also, from this wikipedia article, an example of a DateCite issued IGSN is 10.58052/SSH000SUA
. Note that this is also a DOI.
Per the partnership agreement:
Existing IGSN ID handles will now be registered IGSN ID
DOIs and the handles aliased to the DOIs to ensure that
these continue to resolve.
I take this to mean that there are now DataCite-issued DOIs for all IGSNs, including those historical IGSNs issued before the partnership.
Therefore, I believe NeXus can use the same syntax for DOIs and IGSNs.
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 agree, this was the older syntax. I added some text describing this change:
Since 2021, IGSNs are issued by DataCite, meaning that hat there are now DataCite-issued DOIs for all IGSNs, including those historical IGSNs issued beforehand. Therefore, the syntax is the same as for DOIs.
Syntax: 10.XXXX/XXXXXX
Example: 10.1107/S1600576714027575
base_classes/NXobject.nxdl.xml
Outdated
An ISNI is made up of 16 digits, the last character being a check character. The check character may be either a decimal digit | ||
or the character “X”. | ||
|
||
Syntax: https://isni.org/isni/{ISNI-ID} |
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.
Do we really need the https://isni.org/isni/
prefix?
Couldn't we store the ISNI-ID number and document that a URL may be derived from the ISNI by adding the prefix?
i.e.,
Syntax: 16 base-10 digits stored without any spaces.
Apparently, specifying the lack of spaces is important.
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.
Sounds good. I used
A URL can be generated from the ISNI ID by combining it with the prefix https://isni.org/isni/, resulting in
https://isni.org/isni/{ISNI-ID}.
Syntax: 16 base-10 digits stored without any spaces.
Example: 0000000121032683
base_classes/NXobject.nxdl.xml
Outdated
|
||
An ISSN is an 8-digit unique identifier used to distinguish a serial publication, whether in print or electronic form. | ||
|
||
Syntax: ISSN XXXX-XXXX |
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.
As above, I don't think the prefix ISSN
in the syntax adds anything useful as the type
attribute already identifies the value as an ISSN.
Also (borrowing from Wikipedia) the syntax might be better expressed as NNNN-NNNC
where N is in the set {0,1,2,...,9}, a decimal digit character, and C is in {0,1,2,...,9,X};
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.
Sounds good, I aded text describing that the last digit is a check character and changed the syntax as you suggested.
base_classes/NXobject.nxdl.xml
Outdated
Persistent identifiers are also known as PIDs. | ||
</doc> | ||
<attribute name="type" type="NX_CHAR"> | ||
<doc>The type of identifier used.</doc> |
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 think it would make sense to include a comment about using the most specific type when describing the identifier.
For example, all IGSNs are DOIs and all DOIs are Handles; however, an IGSN should have type IGSN
(and not DOI
or Hdl
).
Similarly, an ARK, Purl, ORCID and ROR identifiers should have their corresponding types and should not use the more generic URL identifier.
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.
fully agreed, I added a recommendation along the lines you describe here
</item> | ||
<item value="URN"> | ||
<doc> | ||
Uniform Resource Name |
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.
We probably should mention that identifiers with more specific type
attribute values should not be stored as a URN, even when this is valid.
The two example that springs to mind are DOI and ISSN, but there may be others with valid URN namespaces.
The doi
URN namespace has been registered, so the URN doi:10.1107/S1600576714027575
is a valid URN-based representation for the DOI 10.1107/S1600576714027575
.
Similarly, the ISSN
URN namespace has been registered, so the URN URN:ISSN:1234-1231
is a valid URN that refers to the ISSN 1234-1231
.
However, I believe we shouldn't allow DOIs (and IGSNs) and ISSNs to be written using the URN
type, but rather with their more specific types.
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.
Added
It is recommended that identifiers with more specific type attribute (such as DOI or ISSN) values should not be stored as a URN, even when this is valid. As an example, the URN doi:10.1107/S1600576714027575 is a valid URN-based representation for the DOI 10.1107/S1600576714027575, but it is strongly recommended to use type="DOI" in this case.
``SAS_`` attributes reserved for use by canSAS https://www.cansas.org | ||
``SILX_`` attributes reserved for use by silx https://www.silx.org | ||
============ ================== ============================================ ============================================================= | ||
reserved prefixes; identifier |
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.
It looks like there are some formatting changes.
IMHO, it would be better to separate out layout/formatting changes from content-modifying changes. This patch should add the identifier
row, but not modify the existing rows (if possible).
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 basically just changed the length of the ============
part to accomodate identifier
. Unfortunately, the doc build currently does not work (because of the new nameType="partial"
), but we can certainly check afterwards if that formatting change is absolutely neccessary.
f7e3efd
to
95bdb03
Compare
CI/CD failing because multi-line doc handling is not implemented (see #1491) |
95bdb03
to
82e2136
Compare
Implements vote in #1451 (comment).
Depends on #1485
Any implementation should consider comments in #1416