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.

Sign up for GitHub

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

Jump to bottom

[WIP][ENH] improve guidelines #28

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

[WIP][ENH] improve guidelines #28

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f1b6f2a
improve guidelines
Remi-Gau Feb 6, 2024
21b5d96
reorder steps
Remi-Gau Feb 19, 2024
d9d26ea
link to examples
Remi-Gau Feb 19, 2024
c02a16a
list of steps to develop BEP
Remi-Gau Feb 19, 2024
6ea4d10
Merge branch 'main' into guidelines
Remi-Gau Feb 28, 2024
f092c4d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 28, 2024
02bca73
Two minor typo fixes
yarikoptic Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 48 additions & 31 deletions docs/guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,74 +40,91 @@ If something being added to BIDS is applicable to at least 80% of use cases

## When and how to start a BIDS Extension Proposal?

Small contributions (typos, rephrasing of a description, adding a single new
metadata field) can be just added as a
[Pull Request on GitHub](https://github.com/bids-standard/bids-specification/pulls)
!!! warning "Does this change need to be a BEP?"

Larger contributions that are expected to involve longer and more involved
discussions should be first described in a standalone document: a Google Docs
BEP. Development on Google Docs is preferred as this is a low barrier to entry
for colleagues who do not use GitHub and/or Markdown,
allowing more people to get involved.
Small contributions
(typos, rephrasing of a description, adding a single new metadata field)
can be just added to the BIDS specification
as a [Pull Request on GitHub](https://github.com/bids-standard/bids-specification/pulls)

1. Explore [the specification](bids-specification.readthedocs.io)
and [the BEP lists](https://bids.neuroimaging.io/get_involved.html#extending-the-bids-specification)
to find existing or ongoing efforts
that may support what you are trying to add into the BIDS Specification.
Someone may have already done work for you.
Larger contributions that are expected to involve longer and more involved discussions
should be first described in a standalone document: a Google Docs BEP.
Development on Google Docs is preferred as this is a low barrier to entry
for colleagues who do not use GitHub and/or Markdown,
allowing more people to get involved.

2. Read the
[BIDS governance document](https://bids.neuroimaging.io/governance.html).
1. Check [the lists of opened BEPs](https://bids.neuroimaging.io/get_involved.html#extending-the-bids-specification)
to find existing or ongoing efforts that may overlap with your idea.
Someone may have already done work for you.

3. Familiarize yourself with the BIDS community by browsing current issues,
discussions, and proposed changes on
[the BIDS specification repository].
1. Check the [opened issues about BEPs](https://github.com/bids-standard/bids-specification/issues?q=is%3Aissue+is%3Aopen+label%3ABEP),
Search for issues relating to your feature or BEP idea
before creating a new issue.

4. Open an initial “issue” on
[the BIDS specification repository] issues page
1. Open an initial “issue” on
the BIDS specification repository [issues page]
to gauge interest in your potential BEP, and to collect
feedback by more community members and
[BIDS maintainers](https://github.com/bids-standard/bids-specification/blob/master/DECISION-MAKING.md#maintainers-group).
**This is an important step before proceeding in order to make sure that
more consensus arises and more contributors are aware what is happening**.

!!! warning

This is an important step before proceeding in order to make sure that
more consensus arises and more contributors are aware what is happening.

5. Communicate with the BIDS maintainers to make your BEP official. This
entails registering the BEP with a number on
1. Communicate with the BIDS maintainers to make your BEP official.
This entails registering the BEP with a number on
Comment on lines +75 to +76
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From discussion with Camille:

It is unclear what is in the internal process or criteria that says that a BEP can get a number.

from the POV of the contributor what would make them going from opening an issue to asking for an official number.

ACTION: maintainers and steering to clarify this

Copy link
Member

Choose a reason for hiding this comment

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

We never formalized this, but from my perspective, BEP leads need to:

  1. open an issue on the spec repo and announce their intent, asking for community (/maintainer/steering) feedback
  2. wait until such feedback comes (e.g., in the form of "we have something similar already, see here", or in the form of "this seems to be a niche case, are you sure that enough people care?", or in the form of "wonderful! We've been needing this!")
  3. get started with a google doc and formalize who are the beps and contributors (if any yet)

Once these three steps are fulfilled, I'd be happy to give the BEP a number and ask the leads to please announce the initiative far and wide.

Copy link

Choose a reason for hiding this comment

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

Thanks @sappelhoff for fleshing this out! I do think this is important and we should get consensus (at minima amongst maintainers + steering?) and make those explicit. Maybe a point to discuss at the next steering/maintainer meetings?

One question for me here is step 2 "wait until such feedback comes" --> should we commit that someone on BIDS governance (a maintainer / a steering member / someone else) will give feedback? i.e. what happens if no feedback comes?

Copy link
Member

Choose a reason for hiding this comment

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

i.e. what happens if no feedback comes?

there have been situations in the past where not a lot of feedback came. I think one example is the suggested BEP that came our of the earlier "cancelled" MIDS BEP:

there is just not a lot happening there and I could understand if the current lead of that "pre-BEP" would be kind of frustrated.

I don't really have a solution for it. Sometimes people also suggest ideas that don't really resonate with anyone, and then, no interaction may also be an answer in itself? But we should figure out more respectful ways of dealing with this.

Copy link
Contributor

@oesteban oesteban Feb 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we should figure out more respectful ways of dealing with this.

I agree this needs to be dealt with, but I would not frame it in terms of what is respectful or not, but rather what ensures that the work done is retrievable if interest returns and is formalized in a way that we retain what was learned (e.g., after all the work we realized it was not the best way of solving something).

I think having zombie BEPs is the worst (least respectful) option. As long as the BEP lifecycle is clear, if a BEP needs to be called "cancelled", I think that's okay (preserving it FAIR and provenance traced).

Copy link
Member

Choose a reason for hiding this comment

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

There are two topics in this thread, on 1) perhaps a this can be something like:

  1. Open an initial “issue” on the BIDS specification repository [issues page] to gauge interest in your potential BEP, and to collect feedback by more community members and BIDS maintainers. Tag several BIDS community members with relevant expertise to comment, and ask whether they can provide feedback of the following form:
    (a) We have something similar already, see here.
    (b) This seems to be a niche case, are you sure that enough people care?
    (c) Wonderful, we've been needing this!
    (d) I do not have sufficient experience/time to comment on this topic now (perhaps tag someone else instead)

on 2) I would prefer 'shelving' rather than 'cancelling' zombie BEPs. This makes it clear that if community interest resurfaces or new leads emerge, they can be taken of the shelve.

[the BIDS website](https://bids.neuroimaging.io/get_involved.html).
To obtain a number for your BEP, follow the previous steps and then [open a
new issue](https://github.com/bids-standard/bids-website/issues)
or [submit a pull request](https://github.com/bids-standard/bids-website/pulls) to the
[website GitHub repository](https://github.com/bids-standard/bids-website/),
cross-linking to any other already existing issues.

6. Create a draft of your extension by discussing among colleagues. The
<!-- 1. Explore [the specification](bids-specification.readthedocs.io)
and [the BEP lists](https://bids.neuroimaging.io/get_involved.html#extending-the-bids-specification)
to find existing or ongoing efforts
that may support what you are trying to add into the BIDS Specification.
Someone may have already done work for you. -->

<!-- 1. Read the [BIDS governance document](https://bids.neuroimaging.io/governance.html). -->

## Developping the BIDS extension proposal
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved

1. Create a draft of your extension by discussing among colleagues. The
[BIDS Extension Proposal template](https://docs.google.com/document/d/1W7--Mf3gCCb1mVfhsoRJCAKFhmf2umG1PFkyZ1jEgMw/edit#)
provides some boilerplate and formatting conventions.

7. List on the draft the contributor(s) leading the effort.
1. List on the draft the contributor(s) leading the effort.

8. Share the draft (remember to
1. Share the draft (remember to
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this supposed to be a google draft or to refer to the pull request?

[share a link that allows anyone to comment](https://support.google.com/docs/answer/2494822?co=GENIE.Platform%3DDesktop&hl=en))
with the
[bids-discussion mailing list](https://groups.google.com/forum/#!forum/bids-discussion)
and ask for comments.

9. Incorporate the feedback and strive for consensus.
1. Incorporate the feedback and strive for consensus.

10. Help to merge the extension into the main specification (this will require
1. Help to merge the extension into the main specification (this will require
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When are people supposed to move from google doc to start using markdown / github to open a pull request ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should they submit only markdown?
Or should they to a certain extend take care of the schema aspect of things + mkdocs macro?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would argue that

  • people should be allowed to develop BEP directly as a PR without going through google doc
  • it might be recommended to work on BEP while preparing the PR reflecting current state in google doc at any point in time: this would allow to
    • see how "intrusive" BEP is -- is it easy to represent it in current schema's model?
    • see if it is "typical", as if e.g. adding new datatype does not require overloading some common entities present in a similar class etc
    • validate sample datasets using new bids-validator which could use the schema from BEP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that the governance mentions a google doc step, I think it is hard for now to say that people can skip this.

https://bids.neuroimaging.io/governance.html#b-standard-decision-making-process-overview

Starting to think we may have to amend the governance too on the next election to clarify our process.

Copy link
Contributor

Choose a reason for hiding this comment

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

I totally agree with @Remi-Gau's initial question. In a private conversation that ended up in #29, I had mentioned this problem and suggested two scenarios:

  1. When the Google Docs document becomes unmanageable by browsers (this happens when there are sufficient numbers of contributors and comments in the history). The BEP Leads could evaluate whether this moment has arrived every month since the start of the Google Doc.
  2. When the Google Docs has not tracked new changes (changes, not comments) for a period of NN months (I'd say 3 or 4?).

Here, it would also clearly state how the migration from GDocs to GH should happen:

  1. Block the GDocs so that no new changes can happen during migration and afterwards.
  2. Whatever it is decided about Remi's questions above: https://github.com/bids-standard/bids-extensions/pull/28/files#r1494306610

Copy link
Contributor

Choose a reason for hiding this comment

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

re @Remi-Gau 's last comment on inter-dependency with governance principles:

  • governance principle is already "buggy" since BEP migration from google doc to PR already includes not only changes to markdown but also to the schema (yaml files)
  • I have now submitted Remove (now outdated/incomplete) going from Google doc to markdown file bids-website#373 to remove "markdown" specific, keeping overall "Google doc --> PR"
  • I will provide suggestion below on that as well
  • more thought is needed indeed to how adjust governance and instructions here. I think keeping BEPs hostages of Google drive can be a disadvantage/unnecessary burden in a number of cases since preparing PR right away encourages desired qualities (review of schema for needed changes which requires better familiarization with BIDS internals, development of example etc)

converting the proposal to Markdown and submitting a Pull Request at
[the BIDS specification repository])
Comment on lines +109 to 111
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. Help to merge the extension into the main specification (this will require
converting the proposal to Markdown and submitting a Pull Request at
[the BIDS specification repository])
1. Help to merge the extension into the main specification.
This will require converting the proposal to changes in [the BIDS specification repository] (schema YAML files, text Markdown files, etc) and submitting a Pull Request.


11. Create example datasets.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Give more specific info on opening PR on the bids examples.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure why this change. We do talk about PR against bids-specification above, so why not to talk about examples? hence I would have returned back (can't add suggestion on deleted line)

1. Create a Pull Request with example dataset(s) for [bids-examples] repository.
   Those examples should pass BIDS Validator when using modified BIDS Specification schema provided in the aforementioned Pull Request for [the BIDS specification repository].

1. Create example datasets.

12. If necessary, contribute a pull request to the
!!! Note
In some cases, examples for the BEP can be designed earlier in the BEP process.
This can help make things more concrete and make it more obvious
what aspects of the BEP work and what aspect may need to be reworked.

1. If necessary, contribute a pull request to the
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that now all validation rules should be encoded in the schema, I am not sure even if this rule remains pertinent. I would make it a little clear(er) that most likely this is not needed

Suggested change
1. If necessary, contribute a pull request to the
1. If necessary (should rarely be necessary), contribute a pull request to the

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this should say how (and/or who) the need for changes in the validator is determined, as opposed to grading the rarity of the event.

Copy link
Contributor

Choose a reason for hiding this comment

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

could you suggest wording to express your idea?

Copy link
Contributor

Choose a reason for hiding this comment

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

done above (I think GH will not reorganize the timeline of my review) or below (if the review order is changed)

[BIDS Validator repository](https://github.com/bids-standard/bids-validator)
as well to incorporate the extension.
Comment on lines +113 to 115
Copy link
Contributor

Choose a reason for hiding this comment

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

@yarikoptic something along these lines (will probably require making consistent with the above line about examples):

Suggested change
1. If necessary, contribute a pull request to the
[BIDS Validator repository](https://github.com/bids-standard/bids-validator)
as well to incorporate the extension.
1. Submit at least one example dataset to the
[BIDS-examples](https://github.com/bids-standard/bids-examples) repository.
All uploaded examples MUST successfully pass the BIDS-Validator.
If the BIDS-Validator requires modifications beyond the updates to the schema
done within the BEP, contribute a pull request to the
[BIDS Validator repository](https://github.com/bids-standard/bids-validator).
If the BEP does not include examples, or there is uncertainty about whether
the BIDS-Validator will properly operate on more complex cases, at least
one maintainer of the BIDS-Validator project MUST accept the proposed
changes or request changes to the BIDS-Validator before acceptance.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for taking a stab at it! "explicit better than implicit" so overall I like the "spirit" of presentation here but IMHO it is too verbose here.
Quite often verbose presentation of details leads to duplication and thus outdated instructions, inconsistencies (e.g. governance doc says one, another one says another) etc. So IMHO it is better to keep it more succinct and to the point, not unlike prior instructions/steps. Then each individual component (e.g. bids-examples or bids-validator) repositories would be a location for ultimate instructions (relates on acceptance of PRs). Moreover BEP might not need a new dataset but might require changes/extensions to existing ones so I might be better to account for that... what about middle ground (modifying your proposed one):

Suggested change
1. If necessary, contribute a pull request to the
[BIDS Validator repository](https://github.com/bids-standard/bids-validator)
as well to incorporate the extension.
1. Submit a pull request (e.g. with an example dataset) reflecting changes proposed by BEP to the
[BIDS-examples](https://github.com/bids-standard/bids-examples) repository.
All additions or changes to example datasets MUST successfully pass the BIDS-Validator using BEP schema.
If the BIDS-Validator requires modifications beyond the updates to the schema
done within the BEP, contribute a pull request to the
[BIDS Validator repository](https://github.com/bids-standard/bids-validator).

Copy link
Contributor

Choose a reason for hiding this comment

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

Heads up - this sentence seems cut off. I would suggest a change, but I don't really know what this is meaning to say:

Suggested change
as well to incorporate the extension.
as well to incorporate the extension.


![bep_process](assets/img/bep_process.png)




Comment on lines +119 to +121
Copy link
Contributor

@yarikoptic yarikoptic Mar 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

unnecessary, not pertinent changes

Suggested change

(all 3 should be deleted, diff seems to show only 1... dunno)

## Advice for extending BIDS

### Limit flexibility, consider tool developers
Expand Down
2 changes: 1 addition & 1 deletion mkdocs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@ markdown_extensions:
anchorlink: true
- pymdownx.superfences
- pymdownx.details
- admonition
- admonition