search

bids-standard / stats-models

BIDS Stats Models Specification
https://bids-standard.github.io/stats-models
Other
2 stars 9 forks source link

should all nodes be in edges (if edges exist)? #66

Open Remi-Gau opened 2 years ago

Remi-Gau commented 2 years ago

All nodes mentioned in edges should refer to an existing node, but I am not sure about the other way around: have some nodes that exist but not mentioned in edges and that would thus be ignore when running the model.

Sounds practical from a user perspective to be able to develop some models where not all nodes are used while tweaking things around.

But validation could possibly throw a warning.

effigies commented 2 years ago

Definitely seems warn-worthy.

Remi-Gau commented 2 years ago

should I just add a validator in the BIDSStatsModel class ?

effigies commented 2 years ago

I guess those could warn instead of raising exceptions, but if they aren't easy to skip, it will make it annoying to program with this class, as every copy will emit a new warning.

Remi-Gau commented 2 years ago

ah true!

so maybe something like validate method on the Edge class?

effigies commented 2 years ago

No, I think it still needs to operate on the whole model, as it needs to know about both nodes and edges. I can see a couple approaches.

  1. An explicit check() method or function that will emit warnings (could have a strict mode to make errors)
  2. Have an explicitly warning subclass of BIDSStatsModel that implements the validation. The caller then chooses which to use for parsing.