Closed samaloney closed 10 months ago
All modified lines are covered by tests :white_check_mark:
Files | Coverage Δ | |
---|---|---|
sunkit_spex/__init__.py | 100.00% <ø> (ø) |
|
sunkit_spex/constants.py | 88.88% <ø> (ø) |
|
sunkit_spex/emission.py | 92.20% <100.00%> (ø) |
|
sunkit_spex/integrate.py | 96.00% <ø> (ø) |
|
sunkit_spex/io.py | 89.07% <ø> (ø) |
|
sunkit_spex/logging.py | 100.00% <ø> (ø) |
|
sunkit_spex/photon_power_law.py | 97.77% <ø> (ø) |
|
sunkit_spex/sunxspex_fitting/__init__.py | 100.00% <ø> (ø) |
|
sunkit_spex/sunxspex_fitting/data_loader.py | 18.42% <100.00%> (ø) |
|
sunkit_spex/sunxspex_fitting/fitter.py | 54.10% <100.00%> (ø) |
|
... and 10 more |
:loudspeaker: Thoughts on this report? Let us know!.
Mac tests pass, the fail is caused by an issue submitting the codecov report.
Can we close the bug fixes/photon power law before merging this please?
Can we close the bug fixes/photon power law before merging this please?
You reminded me of a conversation with @DanRyanIrish at SPD that this was OK to be accepted from his point of view so, before any larger changes are made to the repo, I'll merge the PR in question. Any other changes down the line can be addressed in the refactoring.
Ok think this is all good to go now, once this is merged will prob have to be a small tidy up PR after we rename the GitHub repo if we want to do that, I presume we do?
Thank you @samaloney! Looks comprehensive. After merging this it might be worthwhile to rename the CT/CI stuff too, and fix its public key issues. Do the "other" test issues mean they just failed to run? Are we sure things will not severely break after the merge?
@settwi unfortunately our test coverage isn't great so other than running the test we have and checking the example notebooks work I'm not sure how to know if anything will break.
@KriSun95 ah thanks I missed some of those especially in the comments, I think I have them all now 🔍
Are we close to merging this? Do we just need another approver?
Yea or I can just merge?
Merge it! Or as the Americans are more likely to say (do): PULL THE TRIGGER!
Rename package to
sunkit-spex
and module tosunlit_spex
[x] Check example notebooks
These can really only be done after this is merged