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

JOSS paper #231

Merged
merged 8 commits into from
May 9, 2024
Merged

JOSS paper #231

merged 8 commits into from
May 9, 2024

Conversation

bhazelton
Copy link
Member

Description

Preparation to submit to JOSS. The compiled paper draft can be seen as an artifact on the paper_draft workflow under Actions.

Motivation and Context

Checklists:

Documentation Change Checklist:

@bhazelton bhazelton added the documentation Improvements or additions to documentation label Feb 8, 2024
@bhazelton bhazelton added this to the JOSS Paper milestone Feb 8, 2024
@bhazelton bhazelton requested a review from aelanman February 8, 2024 16:29
Copy link

codecov bot commented Feb 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (f9b8675) to head (5857afd).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #231   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines         2134      2134           
  Branches       608       608           
=========================================
  Hits          2134      2134           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bhazelton
Copy link
Member Author

@aelanman We'd like your comments on this, you're listed as an author :)

aelanman
aelanman previously approved these changes Feb 8, 2024
Copy link
Contributor

@aelanman aelanman left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the co-author credit!

@bhazelton bhazelton force-pushed the joss_paper branch 2 times, most recently from 4ae7147 to 7dbda81 Compare February 27, 2024 16:00
@bhazelton bhazelton force-pushed the joss_paper branch 2 times, most recently from eed8631 to b4c8cef Compare March 13, 2024 18:20
@bhazelton bhazelton marked this pull request as ready for review May 9, 2024 15:20
@bhazelton bhazelton requested review from jpober and mkolopanis May 9, 2024 18:08
jpober
jpober previously approved these changes May 9, 2024
Copy link
Member

@jpober jpober left a comment

Choose a reason for hiding this comment

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

Looks great. Any idea why checks are failing...?

@bhazelton
Copy link
Member Author

bhazelton commented May 9, 2024

Astropy 6.1 dropped a couple days ago and some new warnings are being generated. I fixed a couple of things in our tests (no non-test code needed to be modified), but most of the warnings are coming from lunarsky, tracked in an issue here: aelanman/lunarsky#27.

@jpober jpober self-requested a review May 9, 2024 18:33
Copy link
Member

@jpober jpober left a comment

Choose a reason for hiding this comment

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

Still a lot of failures, alas.

@bhazelton
Copy link
Member Author

Oh, yes. There's another transient issue I haven't run down where sometimes on mac builds in the install step setuptools_scm can't find git. Which makes no sense because the repo was cloned with git. This is occasionally seen in other repos too. For now I'll just re-run but long term we need to figure it out.

@jpober jpober self-requested a review May 9, 2024 18:44
Copy link
Member

@jpober jpober left a comment

Choose a reason for hiding this comment

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

Warning tests are failing still, not just mac builds.

@bhazelton
Copy link
Member Author

bhazelton commented May 9, 2024

Right, the warning test failures are due to the new astropy warnings in lunarsky. They won't go away on a re-run but I'm hesitant to pin astropy because of warnings.

@jpober jpober self-requested a review May 9, 2024 19:06
Copy link
Member

@jpober jpober left a comment

Choose a reason for hiding this comment

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

Still looks like the failing tests are blocking merging. Do we want to override them?

@bhazelton bhazelton merged commit 6947e25 into main May 9, 2024
32 of 34 checks passed
@bhazelton bhazelton deleted the joss_paper branch May 9, 2024 20:52
@bhazelton
Copy link
Member Author

I re-ran until only the warnings tests were failing, then merged.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants