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

[MISC] add "forward slash" requirement for paths to common principles #867

Conversation

sappelhoff
Copy link
Member

@sappelhoff sappelhoff commented Sep 2, 2021


Throughout BIDS all such paths MUST be specified using the slash character (`/`),
regardless of the operating system that a particular dataset is curated on
or used on.
Copy link
Collaborator

@yarikoptic yarikoptic Sep 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if such section is added, it should specify that paths must not be absolute local paths, thus must not start with /.
Didn't analyze yet, but also something might need to be said then what they are relative to, or that what they are relative to would depend on where used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like all of this would be part of #820 so maybe this PR should be adjusted and merged AFTER #820 is in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some language for that, ... in the meantime, #820 was replaced with #918 - so we should either:

  1. merge this before [ENH] Add BIDS URIs and deprecate relative paths, RawSources and (possibly unused) BasedOn #918 and then potentially adjust the added text here in [ENH] Add BIDS URIs and deprecate relative paths, RawSources and (possibly unused) BasedOn #918, OR
  2. wait until [ENH] Add BIDS URIs and deprecate relative paths, RawSources and (possibly unused) BasedOn #918 is merged, then adjust this one and merge it.

@sappelhoff sappelhoff force-pushed the common_principles/forward-slash-paths branch from a8ed08b to 63f47d1 Compare November 8, 2021 14:38
Copy link
Collaborator

@Remi-Gau Remi-Gau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member Author

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@effigies given that for #918 we want to work on validator and examples some more before merging, I think that this one can be merged first. WDYT? Can you give this a look?

@sappelhoff sappelhoff added this to the 1.7.0 milestone Jan 23, 2022
@sappelhoff sappelhoff merged commit 5fe26e1 into bids-standard:master Jan 31, 2022
@sappelhoff sappelhoff deleted the common_principles/forward-slash-paths branch January 31, 2022 10:36
Lestropie added a commit to Lestropie/bids-specification that referenced this pull request Apr 29, 2022
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants