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

[ENH] use citation.cff to store affiliations of authors #3754

Merged
merged 22 commits into from Jul 4, 2023

Conversation

Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Jun 14, 2023

Relates to #1511

Changes proposed in this pull request:

  • add citation.cff file to store authors name, affiliation, email, orcid...
  • add workflow to validate citation.cff file

@github-actions
Copy link
Contributor

github-actions bot commented Jun 14, 2023

👋 @Remi-Gau Thanks for creating a PR!

Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft.

Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.

  • PR has an interpretable title.
  • PR links to Github issue with mention Closes #XXXX (see our documentation on PR structure)
  • Code is PEP8-compliant (see our documentation on coding style)
  • Changelog or what's new entry in doc/changes/latest.rst (see our documentation on PR structure)

For new features:

  • There is at least one unit test per new function / class (see our documentation on testing)
  • The new feature is demoed in at least one relevant example.

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

We will review it as quick as possible, feel free to ping us with questions if needed.

@Remi-Gau
Copy link
Collaborator Author

Other benefits of citation.cff files: they add "cite this repo" button on github

Screenshot from 2023-06-14 14-58-13

@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Jun 14, 2023

If there is interest for this options the next step would be:

  • add missing info citation.cff (affiliations, orcid...)
  • add a script to generate section of names.rst and AUTHORS.rst from the citation.cff files
  • add automation to make sure that the names and AUTHORS.rst file gets updated when citation.cff is updated
  • update documentation to mention how people should add their info

What this does not cover are all the contributors listed in the names.rst files.

@Remi-Gau
Copy link
Collaborator Author

@bthirion this is what the citation file would look like.

@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Merging #3754 (fe4a835) into main (089a1a0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3754   +/-   ##
=======================================
  Coverage   91.52%   91.52%           
=======================================
  Files         133      133           
  Lines       15561    15561           
  Branches     3229     3229           
=======================================
  Hits        14242    14242           
  Misses        772      772           
  Partials      547      547           
Flag Coverage Δ
macos-latest_3.10 91.44% <ø> (ø)
macos-latest_3.11 91.44% <ø> (ø)
macos-latest_3.8 91.41% <ø> (ø)
macos-latest_3.9 91.41% <ø> (ø)
ubuntu-latest_3.10 91.44% <ø> (ø)
ubuntu-latest_3.11 91.44% <ø> (ø)
ubuntu-latest_3.8 91.41% <ø> (ø)
ubuntu-latest_3.9 ?
windows-latest_3.10 91.38% <ø> (ø)
windows-latest_3.11 91.38% <ø> (ø)
windows-latest_3.8 91.35% <ø> (ø)
windows-latest_3.9 91.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bthirion
Copy link
Member

I think we need to upgrade the authors list to include recent contributors.
Is this much effort to leverage that in the generated doc ?

@Remi-Gau
Copy link
Collaborator Author

I think we need to upgrade the authors list to include recent contributors. Is this much effort to leverage that in the generated doc ?

You mean update AUTHORS.rst with people listed here: https://github.com/nilearn/nilearn/blob/main/doc/changes/names.rst

Copy link
Member

@bthirion bthirion left a comment

Choose a reason for hiding this comment

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

LGTM so far.

@bthirion
Copy link
Member

The doc build failure looks related, isnt'it ?

@Remi-Gau
Copy link
Collaborator Author

The doc build failure looks related, isnt'it ?

yup.
starte looking into but could not understand why in 5 minutes so will get back to it: most likely next week though

AUTHORS.rst Outdated Show resolved Hide resolved
@Remi-Gau
Copy link
Collaborator Author

Random thought: looking up all the affiliations of all contributors may be something that we want to keep as a good first issue for the OHBM hackathon.

It could be a good way for people to who want to practice git and opening pull requests... And teach them about the joy of editing yml files...

@Remi-Gau Remi-Gau marked this pull request as ready for review June 28, 2023 05:27
@Remi-Gau
Copy link
Collaborator Author

make sure AUTHORS.rst and doc/changes/names.rst are regenerated before building the doc in case a contributor adds their names to the change log but forgets to generate doc/changes/names.rst

Comment on lines +261 to +265
# Update the authors file and the names file
# in case a contributor has been added to citation.cff
# but did not run the maint_tools/citation_cff_maint.py script.
- name: update AUTHORS.rst and doc/changes/names.rst
run: python maint_tools/citation_cff_maint.py
Copy link
Member

Choose a reason for hiding this comment

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

Since running citation_cff_maint.py is automated can we keep the instructions simple for 1st time contributors so all they have to do is add their names to citation.cff ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yup I was thinking about that too: will update

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

update the contributing.rst also with an example of what to add in the citation.cff

Copy link
Member

@ymzayek ymzayek left a comment

Choose a reason for hiding this comment

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

LGTM!

@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Jul 4, 2023

ok will merge this one as we will need it for the hackathon

@Remi-Gau Remi-Gau merged commit c8cd50f into nilearn:main Jul 4, 2023
29 checks passed
@Remi-Gau Remi-Gau deleted the enh/citation_cff branch July 4, 2023 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants