astropy / specreduce

Tools for the reduction of spectroscopic observations from Optical and NIR instruments
https://specreduce.readthedocs.io
61 stars 38 forks source link

MNT: Infrastructure and other updates #202

Closed pllim closed 10 months ago

pllim commented 11 months ago

I stared into the code and I couldn't unsee things, so here it is.

~Unable to handle this one until I get access:~

After merge, can add new job names to branch protection.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (aa4ceab) 78.83% compared to head (bbf377d) 81.32%.

Files Patch % Lines
specreduce/fluxcal.py 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #202 +/- ## ========================================== + Coverage 78.83% 81.32% +2.49% ========================================== Files 10 10 Lines 992 996 +4 ========================================== + Hits 782 810 +28 + Misses 210 186 -24 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pllim commented 11 months ago

Close/reopen to see if RTD pops up.

pllim commented 11 months ago

Sorry about the conflict with #182 . I don't watch this repo so I was not aware of it. I replied in https://github.com/astropy/specreduce/pull/202#discussion_r1418278247 (I think it is a little too soon but I am also not a specreduce maintainer).

Whether you want to merge this first, or cherry-pick your changes to this PR and take this over, I will leave to you.

tepickering commented 11 months ago

no worries on the conflicts. i would say let's merge this ASAP and i'll do what i need to do to clean up #182. it's the code changes that i've made in #182 that require 3.10+, iirc. if we want to stick with bumping the requirement there vs here, i'm fine with that. interested in @kecnry's take.

kecnry commented 11 months ago

I don't have any strong opinions on the order of operations - good to merge on my end! Thanks for all the improvements @pllim !

pllim commented 11 months ago

Then, feel free to merge! I am tempted to merge myself but I shouldn't. Thanks!