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

[REF] Replace duecredit with BibTeX #875

Merged
merged 11 commits into from
Aug 12, 2022
Merged

[REF] Replace duecredit with BibTeX #875

merged 11 commits into from
Aug 12, 2022

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Apr 29, 2022

Closes #873.

Changes proposed in this pull request:

  • Remove duecredit as a dependency and remove all due decorators.
  • Remove the RefLGR and have BibTeX citations in the RepLGR strings.
    • We still need to figure out how to load the references BibTeX file, select the references that are actually used in the report string, and output those to the report. Done!
  • Add references.bib BibTeX file.
  • Use sphinxcontrib-bibtex for docstring references.
    • We can also use this throughout the documentation, outside of docstrings!

@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #875 (aab1cdf) into main (a008689) will increase coverage by 0.02%.
The diff coverage is 95.50%.

@@            Coverage Diff             @@
##             main     #875      +/-   ##
==========================================
+ Coverage   93.26%   93.28%   +0.02%     
==========================================
  Files          27       28       +1     
  Lines        2315     2337      +22     
==========================================
+ Hits         2159     2180      +21     
- Misses        156      157       +1     
Impacted Files Coverage Δ
tedana/__init__.py 100.00% <ø> (ø)
tedana/combine.py 90.62% <ø> (-0.81%) ⬇️
tedana/decay.py 97.26% <ø> (-0.02%) ⬇️
tedana/decomposition/_utils.py 0.00% <ø> (ø)
tedana/decomposition/ica.py 82.14% <ø> (-0.62%) ⬇️
tedana/decomposition/pca.py 89.51% <0.00%> (+0.45%) ⬆️
tedana/gscontrol.py 100.00% <ø> (ø)
tedana/io.py 94.00% <ø> (-0.03%) ⬇️
tedana/metrics/collect.py 94.94% <ø> (-0.03%) ⬇️
tedana/metrics/dependence.py 97.72% <ø> (-0.02%) ⬇️
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tsalo
Copy link
Member Author

tsalo commented Jul 21, 2022

@eurunuela this PR is now ready for review, if you have the time for it.

eurunuela
eurunuela previously approved these changes Jul 21, 2022
Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

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

This is great. Thank you @tsalo!

I built the docs on my computer and the references look perfect.

I guess my only question is regarding GitHub's supported citations. Is that something we have to do once the PR is merged?

@tsalo
Copy link
Member Author

tsalo commented Jul 21, 2022

Sorry, what do you mean by "GitHub's supported citations"? Do you mean the "Cite this repository" option available when repositories have a CFF file?

@eurunuela
Copy link
Collaborator

Sorry, what do you mean by "GitHub's supported citations"? Do you mean the "Cite this repository" option available when repositories have a CFF file?

Sorry, yes. That's what I was referring to.

@tsalo
Copy link
Member Author

tsalo commented Jul 21, 2022

We can tackle that in a separate PR. We can add the CFF file with the JOSS paper citation, but I'm still concerned about getting folks to cite individual releases' Zenodo DOIs. The two problems are (1) CFF can support multiple citations, but will only show one of them to users on GitHub, and (2) the Zenodo DOI is minted after the release is made, so any code in tedana will only reflect the previous release.

This PR doesn't address the second problem, but it does let us put as many references as we want in the reports (as we already do), so we could have both the JOSS and Zenodo citations in the boilerplate.

@dowdlelt
Copy link
Collaborator

I haven't had a chance to do an in depth look at this so maybe I'm completely missing the mark...but before I forget: I looked at the docs generated for this PR and duecredit is mentioned right at the beginning under the Citation heading as well as in the Installation section. Should reference to duecredit be removed there (it is also in the FAQ section, but that may be wise if someone is encountering duecredit in an older version).

@tsalo
Copy link
Member Author

tsalo commented Jul 28, 2022

I forgot about the docs. Thanks for catching that! I've removed all mentions of duecredit.

@eurunuela
Copy link
Collaborator

Good catch @dowdlelt! I missed it too.

@jbteves
Copy link
Collaborator

jbteves commented Aug 2, 2022

This looks good, but could we move all of the bibtex utils into a new file, perhaps bibtex.py? I just feel like it will be difficult to figure out how they're distinct from other utilities with the current names, and there's several of them so it makes sense to split them off.

@tsalo
Copy link
Member Author

tsalo commented Aug 8, 2022

@jbteves that's a great idea. I think everything should now be sufficiently separated.

@jbteves
Copy link
Collaborator

jbteves commented Aug 8, 2022

I'm biased because it's my idea but looks good to me 😉

@tsalo tsalo requested a review from eurunuela August 8, 2022 22:34
Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @tsalo

@tsalo
Copy link
Member Author

tsalo commented Aug 12, 2022

Thanks all! Merging now.

@tsalo tsalo merged commit 4b44d55 into ME-ICA:main Aug 12, 2022
@tsalo tsalo deleted the bibtex branch August 12, 2022 14:22
@tsalo tsalo mentioned this pull request Nov 8, 2022
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.

Enhance reports with BibTeX citations
4 participants