[ENH] Encode mutually exclusive extensions in schema with sub-lists #1492
Conversation
| - .raw | ||
| - .ave | ||
| - .mrk |
There was a problem hiding this comment.
.mrk seems to only exist for markers suffix
There was a problem hiding this comment.
yes, some context in this thread: #638 (comment) and in particular this reponse (i.e., why .mrk is only for the markers suffix, but .sqd can also be used for the brain data): #653 (comment)
| text += "\n" + entity_text | ||
|
|
||
| text = text.replace("SPEC_ROOT", utils.get_relpath(src_path)) | ||
| return text | ||
|
|
||
|
|
||
| def _make_entity_definition(entity, entity_info): | ||
| def _make_definition_for_entity(entity_info): |
There was a problem hiding this comment.
just removing some dead code
| @@ -218,6 +218,7 @@ def make_filename_template( | |||
| src_path=None, | |||
| n_dupes_to_combine=6, | |||
| pdf_format=False, | |||
| include_legend=True, | |||
There was a problem hiding this comment.
mostly added to simplify testing
| @@ -162,7 +162,11 @@ def _sanitize_extension(ext: str) -> str: | |||
|
|
|||
| def _stem_rule(rule: bst.types.Namespace): | |||
| stem_regex = re.escape(rule.stem) | |||
| ext_match = "|".join(_sanitize_extension(ext) for ext in rule.extensions) | |||
| if isinstance(rule.extensions[0], list) and len(rule.extensions) == 1: | |||
There was a problem hiding this comment.
started trying to fix the failures of schema code but I think I should let grown ups deal with this
| class Group: | ||
| def __init__(self, extensions): | ||
| self.extensions = extensions |
There was a problem hiding this comment.
probably not the best way to mock a group of suffixes...
|
I would personally consider the rendering fixed and OK but I will let others be the final judge of this.
|
|
@effigies Will let other fixes to you. |
|
on second though this rule should phrased somewhere in the specification and not just in the schema, no? |
effigies
added
the
schema-structure
Update expected number of lines, remove unused tests.
Adjust tests and validator code
effigies
changed the title
Co-authored-by: Chris Markiewicz <[email protected]>
|
xref #1560 |
|
|
test failures started appearing after this commit: |
|
|
||
| extensions = [] |
There was a problem hiding this comment.
needed to add this to make some test pass
@effigies WDYT?
| # Combine exts when there are many, but keep JSON separate | ||
| if ".json" in extensions: | ||
| extensions = [".<extension>", ".json"] | ||
| extensions = [] |
There was a problem hiding this comment.
also this horror had to be added
|
@Remi-Gau I don't know if we want to try to work on this in the nearish future, but it will need merge conflicts to be resolved before we go further. |
|
will try to fix |
|
Okay, I got tests passing, although this does seem to be failing to render a bunch of extensions. Can look into it. Before I do, though,I wonder if what we instead want is mutually exclusive lists of valid groups (instead of lists of mutually exclusive groups): extensions:
- [.nii.gz, .json]
- [.nii, .json]
...
extensions:
- [.eeg, .vhdr, .vmrk, .json]
- [.set, .fdt, .json]
- [.edf, .json],
- [.bdf, .json]Sure, it's duplicative, but it's not as ambiguous as the current proposal, where you could have a We could go further and start referencing extension sets: extensionsets:
brainvision:
required: [.eeg],
optional: [.vhdr, .vmrk],
nifti:
oneOf:
- required: [.nii]
- required: [.nii.gz]
sidecar:
optional: [.json]extensions:
- nifti
- sidecar
extensions:
- [edf, brainvision, eeglab, biosemi]
- sidecar
# OR
extensions:
- [nifti, sidecar]
extensions:
- [edf, sidecar]
- [brainvision, sidecar]
- [eeglab, sidecar]
- [biosemi, sidecar] |
|
yeah I like the idea of the extension groups: may allow some refactoring |
|
Inline extension groups (as in the first proposal) or explicit (one of the latter approaches)? |
|
definitely explicit |
fixes #1487