Closed kevin218 closed 11 months ago
Do you mind if I add in that jwst
version change that I talked about? I've checked that NIRSpec G395H gives similar results to the older jwst
version, and just need to finish checking NIRCam and MIRI. Should be certain that the updated version works for all three instruments by this evening
@taylorbell57 Yeah, go for it.
@taylorbell57 The PR should now be ready for another review.
@kevin218 since it'll be a super quick change, should we address the zero vs one-based indexing issue from #576 in this PR as well?
Ok, I removed the other self.multwhite
from BatmanModels.py and changed the S5 log statement to start from 0. I did not change the total number of channels (i.e., {chanrng}
to {chanrng-1}
) because that would be confusing. For example, if there are 47 channel then the log will record channels 0 to 46 out of 47 channels.
Alright, there's a flake8 issue and also my PR #577 that is setup to merge into this branch (I figured that was a cleaner way of doing things than merging both into main). My PR will also resolve the flake8 issue since I tweaked the S6 code a bit differently
Just running the tests locally after all those edits before approving
Those two return None
changes should be return meta
, right?
Alright, there were several old flake8 issues that I found running flake8 locally (not sure why GitHub's flake8 missed them). I also fixed two bugs that failed my local tests and further fixed the S6 parse function bug. This PR is good to go in my opinion — I'll approve it, and you can merge it if you're happy with my tweaks
I didn't see that you had made changes. I think we're good now.
Let's merge #578 into this first with some more fixes. I'll make one more commit there today after grocery shopping
Merging #575 (f9b91f8) into main (559f703) will increase coverage by
0.62%
. Report is 109 commits behind head on main. The diff coverage is53.14%
.
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
@@ Coverage Diff @@
## main #575 +/- ##
==========================================
+ Coverage 57.02% 57.64% +0.62%
==========================================
Files 94 96 +2
Lines 11282 11547 +265
==========================================
+ Hits 6433 6656 +223
- Misses 4849 4891 +42
Files | Coverage Δ | |
---|---|---|
src/eureka/S3_data_reduction/__init__.py | 100.00% <100.00%> (ø) |
|
src/eureka/S3_data_reduction/background.py | 42.44% <100.00%> (ø) |
|
src/eureka/S3_data_reduction/bright2flux.py | 72.72% <100.00%> (+0.45%) |
:arrow_up: |
src/eureka/S3_data_reduction/straighten.py | 100.00% <100.00%> (ø) |
|
...curve_fitting/differentiable_models/StarryModel.py | 75.00% <100.00%> (+1.40%) |
:arrow_up: |
...ghtcurve_fitting/differentiable_models/__init__.py | 100.00% <100.00%> (ø) |
|
...ureka/S5_lightcurve_fitting/models/ExpRampModel.py | 83.60% <100.00%> (ø) |
|
...S5_lightcurve_fitting/models/SinusoidPhaseCurve.py | 83.14% <100.00%> (ø) |
|
...rc/eureka/S5_lightcurve_fitting/models/__init__.py | 100.00% <100.00%> (ø) |
|
src/eureka/S6_planet_spectra/plots_s6.py | 78.57% <100.00%> (+0.95%) |
:arrow_up: |
... and 32 more |
... and 3 files with indirect coverage changes
:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!
Wow, all checks are finally passing. Nice job, @taylorbell57!
Alright, made two last tweaks to setup.cfg which removes cython there and also fixes a deprecation with license_file
which is now license_files
(see here).
This is a small PR to create the next release. In addition to updating the installation instructions, I have a few small changes to some ECFs.
Just kidding... There's also a bunch of changes from when I fixed
multwhite
.