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

What happens if both /nirs(i)/data(j)/measurementLists and /nirs(i)/data(j)/measurementList are both present #155

Open
rob-luke opened this issue Sep 13, 2024 · 12 comments
Labels
help wanted Extra attention is needed

Comments

@rob-luke
Copy link
Member

rob-luke commented Sep 13, 2024

As asked in the in person meeting

@rob-luke rob-luke added the help wanted Extra attention is needed label Sep 13, 2024
@rob-luke
Copy link
Member Author

As an end user, if both fields are present they should contain the same data and you could load either.

@samuelpowell
Copy link
Collaborator

We should surely enforce that only one is present?

@dboas
Copy link
Collaborator

dboas commented Sep 16, 2024 via email

@samuelpowell
Copy link
Collaborator

@dboas if we specify that only one should be present, then compliant software will never write both to a file, and hence the user will never encounter this problem, right?

@dboas
Copy link
Collaborator

dboas commented Sep 19, 2024 via email

@sstucker
Copy link
Collaborator

Because the above conclusion did not make it into writing in the specification, I did not write the validator this way. Will support whatever you decide and put into the spec itself.

@dboas
Copy link
Collaborator

dboas commented Jan 2, 2025

@samuelpowell
how do you suggest the specification language indicate that only one be present?
Presently they both indicate that they are required if the other is not present.
I am not sure how we would want to write it so that the validator can parse it properly.

@sstucker , given you just worked on updating the validator to handle this one of the two must be present condition, do you have an idea of what language we could use to make it clear that only one should be present so that it is clear to the human, and clear to the validator?

@samuelpowell
Copy link
Collaborator

Presently they both indicate that they are required if the other is not present.

It might be a touch enigmatic but surely this is sufficient insofar as it is logically consistent and I assume parseable?

@dboas
Copy link
Collaborator

dboas commented Jan 3, 2025

Sam is right. The validator could handle this and throw an error if both are present. We could clarify the language in the spec to indicate to the human that we mean that only one can be present.
@sstucker , could we change
Presence: required if measurementLists is not present
to be
Presence: required if measurementLists is not present, but should not be present if measurementLists is present.
without messing up the validator?

Of course we would do similar for the measurementLists Presence requirement.

@samuelpowell
Copy link
Collaborator

@dboas @sstucker for the purpose of simplifying parsing, perhaps the human readable part could be parenthesised, e.g.,

Presence: required if measurementLists is not present (but should not be present if measurementLists is present)

@dboas
Copy link
Collaborator

dboas commented Jan 14, 2025

@samuelpowell and @sstucker ,
@sreekanthkura7 pointed out to me that he is pretty sure that Stephen's validator is using the coding in the SNIRF data format summary. He needs to verify this and will confirm for us here. But Stephen could also verify if he beats Sreekanth to it.

If this is true, Sreekanth and I were thinking that we can code this language of one or the other being present, but NOT both at the same time, but using '*abc' where 'abc' is some letter as opposed to what is presently defined as '*n' where n is a number is this says that atleast oneof the sub-fields must be present must be present. We could instead be mores distinction and instead use '**n' or '**abc' or '@n'
image

Any thoughts on this?

@samuelpowell
Copy link
Collaborator

I think this is true @dboas and indeed an exclusive OR operator such as you propose makes sense. Maybe ^ is a good option given it is a (bitwise) XOR operator in C etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants