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

DOC: Update citation file with JOSS paper reference #496

Merged
merged 21 commits into from
Nov 2, 2021

Conversation

Lucia-Fonseca
Copy link
Member

@Lucia-Fonseca Lucia-Fonseca commented Sep 10, 2021

Description

This PR updates the citation guidelines to include the JOSS paper and link the rst file to the documentation page. Merging this PR successfully closes #495 .

Checklist

  • Follow the Contributor Guidelines
  • Write unit tests
  • Write documentation strings
  • Assign someone from your working team to review this pull request
  • Assign someone from the infrastructure team to review this pull request

@Lucia-Fonseca Lucia-Fonseca added documentation Improvements or additions to documentation enhancement Improvement of existing feature joss_paper labels Sep 10, 2021
@Lucia-Fonseca Lucia-Fonseca self-assigned this Sep 10, 2021
@Lucia-Fonseca
Copy link
Member Author

This is done. Can anyone check the bib file and the Zenodo reference therein? Thank you!

@Lucia-Fonseca
Copy link
Member Author

Lucia-Fonseca commented Sep 10, 2021

Docs are failing but it was working fine on my computer

@rrjbca
Copy link
Contributor

rrjbca commented Sep 10, 2021

Docs are failing but it was working fine on my computer

You should add developer/citation under the developer toctree in index.rst. Although I would suggest moving citation.rst into docs/project as this seems more appropriate than docs/developer

@Lucia-Fonseca
Copy link
Member Author

The Download BibTeX button is not working. Any ideas? It might be spaces but I cannot make it work consistently

CITATION.rst Outdated Show resolved Hide resolved
@Lucia-Fonseca
Copy link
Member Author

Is there a better place for the main.bib file?

@Lucia-Fonseca
Copy link
Member Author

Ready for review

@rrjbca
Copy link
Contributor

rrjbca commented Sep 10, 2021

I don't think having a bib file and a download link is a good idea:

@Lucia-Fonseca
Copy link
Member Author

I don't think having a bib file and a download link is a good idea:
Oh, I liked it but yes, it does not render in GitHub.

@ntessore
Copy link
Member

We could display the JOSS badge somewhere as well: DOI

@rrjbca
Copy link
Contributor

rrjbca commented Sep 10, 2021

We could display the JOSS badge somewhere as well: DOI

It's already on the README. Do you want it in the docs also?

@ntessore
Copy link
Member

Just an idea, I like that big green box. On the other hand, someone on the CITATION page has already clicked and gone there on purpose, so perhaps not necessary to decorate the page any further.

@rrjbca
Copy link
Contributor

rrjbca commented Sep 10, 2021

It could work, instead of the current text which is a bit wordy and clumsy we could have:

The BibTex entries can be found at the following links:
DOI DOI

@Lucia-Fonseca
Copy link
Member Author

The badges
JOSS Zenodo

do not render well in the docs

CITATION.rst Outdated Show resolved Hide resolved
CITATION.rst Show resolved Hide resolved
@Lucia-Fonseca
Copy link
Member Author

Ready

the following acknowledgment:

This research made use of SkyPy, a Python package for forward modeling
astronomical surveys (SkyPy Collaboration, 2020).
astronomical surveys (Amara et. al., 2021, SkyPy Collaboration, 202x).
Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies if I have missed discussions which already address these points.

  • Are we happy with 'Amara et al' or would another 'SkyPy Collaboration' be the preferred style? I appreciate there are probably reasons for this I have missed!
  • We could do the work for the user of including the most up to date version year/reference, which I guess would just require changing as part of each new release.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Are we happy with 'Amara et al' or would another 'SkyPy Collaboration' be the preferred style? I appreciate there are probably reasons for this I have missed!

"SkyPy Collaboration" isn't listed as an author on the JOSS paper, their template wouldn't allow it. Are you suggesting that people cite it as SkyPy Collaboration 2021 regardless? Would that work with ADS etc?

  • We could do the work for the user of including the most up to date version year/reference, which I guess would just require changing as part of each new release.

But then if somebody is using an older version they shouldn't be citing the most up to date version. It might be possible that for each release the published documentation give the corresponding DOI, but only if we can reserve zenodo DOIs before me make the release. As our Zenodo admin could you investigate this?

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 am not happy with Amara et al. either, if we could do SkyPy Collaboration and not mess up with ADS I would go for it.

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 cannot find anything in the ADS FAQ to answer this...

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is possible to reserve a zenodo DOI when using the github webhook, so we would have to revert to manual submissions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this something we want to try?
I'd rather have SkyPy Collaboration rather than Amara et al. 2021 but:

  • Is it worth delaying the release?
  • Sutieng's paper is ready and they cite Amara et al 2021.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, releasing this PR is our single highest priority. The proposed guidelines are adequate and these side issues are unresolved and unnecessarily holding us up. Like it or not, Amara et al 2021 is the correct citation given by both ADS and JOSS. I agree that our citation guidelines should be consistent with ADS in the first instance. I'm also against modifying the ADS entry at the expense of consistency with the entry on the JOSS website. Similarly, reserving a DOI has not been tested with the GitHub webhook and putting the wrong DOI in our citation guidelines could be incredibly harmful. I would suggest trying to reserve a DOI for our next release and if we can demonstrate that it works then integrate reserved DOIs into the citation guidelines for future releases.

Copy link
Member

Choose a reason for hiding this comment

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

I don't disagree necessarily with any of this, but I think it is not fair labeling this a "side issue" or "unnecessary". We agreed pretty early on that all publications would be authored by "SkyPy collaboration" with an alphabetical author list following. Now reneging on this merits at least a discussion, and probably a much more visible one than hidden in this PR somewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Totally agree these are decisions beyond the scope of this PR and review that should be discussed by the collaboration. But the current proposal is at least accurate and waiting for the resolution of that discussion could cost us citations. The guidelines can always be updated in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, we'd need a conversation with the collaboration and we can also merge this as it is for the sake of the v0.5 release. After all, the issue is not about the author list (which I agree it's not a secondary issue but it is not a GitHub issue either)

CITATION.rst Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
@ntessore ntessore removed their request for review September 14, 2021 15:48
@rrjbca rrjbca added the high priority issues needing immediate attention label Sep 16, 2021
CITATION.rst Outdated Show resolved Hide resolved
rrjbca
rrjbca previously approved these changes Sep 16, 2021
Copy link
Contributor

@rrjbca rrjbca left a comment

Choose a reason for hiding this comment

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

I'm happy to approve this PR in it's current state (although possibly check my brief comment r.e. unformatted pull-quote) but will wait for others involved to approve before merging.

@rrjbca rrjbca added this to the Version 0.5 milestone Sep 20, 2021
@Lucia-Fonseca
Copy link
Member Author

After offline conversations, we agree the citation would be Amara et al. 2021 and not the preferred option SkyPy Collaboration 2021 for justified reasons. Therefore, this PR can be merged.

@rrjbca rrjbca merged commit 5162441 into skypyproject:main Nov 2, 2021
itrharrison added a commit that referenced this pull request Dec 15, 2022
* Update name of default branch to main (#434)

* update mailmap (#432)

* Write all tables to a single FITS/HDF5 file (#425)

* ADR 3: Position sampling and patches of the sky (#422)

* BUG: Raise ImportError if optional dependency speclite is not installed (#437)

* MAINT: Set NumPy latest supported version to 1.20 #440

* Update status badges (#441)

* MAINT: Update Lucia affiliation (#451)

* MAINT: add SIT's information (#450)

* DOC: Fix contributor guidelines link (#449)

Co-authored-by: Richard R <[email protected]>
Co-authored-by: Nicolas Tessore <[email protected]>

* ENH: Logging for skypy command line script and Pipeline class (#453)

* DOC: Describe speclite filters in documentation (#457)

* ENH: Config syntax for importing objects (#463)

* DOC: List of Features (#456)

* DOC: How to construct config files (#454)

* DOC: Remove docstring examples (#429)

* MAINT: Update Zenodo entry for RPR (#468)

* DOC: Readme updates (#460)

* DOC: Expanded landing page documentation (#228)

* DOC: Inverse transform sampling accuracy warning (#472)

* MAINT: Set astropy latest supported version to 4.2 (#483)

* DOC: zenodo json members update (#481)

* DOC: Ryden04 ellipticity doc missing section (#477)

* MAINT: Update numpy and scipy latest supported versions (#488)

* BUG: Change invalid ecsv datatype from int to uint16 (#485)

* DEV: setuptools==58.0.0 (#493)

Co-authored-by: Nicolas Tessore <[email protected]>

* Add compatibility workflow badge (#487)

* DEV: Enable pip to install pre-releases in the tox dev environments (#491)

* TST: Use tmp_path fixture for temporary files in unit tests (#489)

* BUG: Move handling of context arguments after handling of .depends keyword (#465)

* BLD: Set astropy latest supported version to 4.3 and speclite minversion to 0.14 (#486)

* REV: restore setuptools to latest version on readthedocs (#494)

* DEV: pyparsing<3.0.0 (#500)

* Check new astropy file overwrite error message in logging test (#498)

* REV: restore pyparsing to latest version for doc builds (#501)

* DOC: Update citation file with JOSS paper reference (#496)

* BLD: Set astropy latest supported version to 5.0 (#504)

* BLD: Set python latest supported version to 3.10 (#505)

* BLD: Set numpy latest supported version to 1.22 (#506)

* BLD: Set python oldest supported version to 3.7 (#507)

* DOC: Fix code of conduct link (#508)

* Changed y-label in luminosity function example. (#512)

* BLD: Set scipy latest supported version to 1.8 (#510)

* ENH: Rykoff model of the magnitude uncertainty (#526)

* TST: assert photometric error is numerically close to the analytic value (#545)

* TST: Drop deprecated astropy.tests.helper.raises (#546)

* ENH: compute kcorrect remaining stellar mass (#476)

* compute kcorrect remaining stellar mass

* added test for stellar mass remain

Co-authored-by: Ian Harrison <[email protected]>
Co-authored-by: Lucia F. de la Bella <[email protected]>

* ENH: Logistic completeness function (#521)

* BLD: Set astropy latest supported version to 5.1 (#547)

* BUG: `schechter_smf` callable input and docs (#525)

* DOC: Typo in Rykoff error (#550)

* add Fox's details (#551)

Co-authored-by: Richard R <[email protected]>

* BLD: Set numpy latest supported version to 1.23 (#552)

* codestyle fixes

* add six requirement for colossus

* tried to fix docs builds

* update passenv

* rtd configuration

Co-authored-by: Richard R <[email protected]>
Co-authored-by: Nicolas Tessore <[email protected]>
Co-authored-by: Lucia F. de la Bella <[email protected]>
Co-authored-by: Sut-Ieng Tam <[email protected]>
Co-authored-by: philipp128 <[email protected]>
Co-authored-by: Fox Davidson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement Improvement of existing feature high priority issues needing immediate attention joss_paper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: update citation guidelines and link to docs
4 participants