Skip to content

DM-56184: Various fixes from Rubin - #187

Merged
jeremyneveu merged 6 commits into
LSSTDESC:masterfrom
lsst:tickets/DM-56184
Sep 22, 2026
Merged

jeremyneveu merged 6 commits into
LSSTDESC:masterfrom
lsst:tickets/DM-56184

Conversation

@timj

@timj timj commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

@jeremyneveu these are the changes from our fork that we discussed on the RFC. Please can you take a look at them and hopefully accept them:

  • Fixes the table file
  • Various numpy warnings fixed
  • Matplotlib fixes (we can not have a library unconditionally set a backend) or display content.

cc/ @erykoff and @mfisherlevine (who wrote these fixes)

mfisherlevine and others added 6 commits September 21, 2026 15:57
Loading parameters.py will no longer try to set the backend. Instead, it will turn off the display mode if the env var is not set or if the matplotlib backend is set to a static (non-interactive) backend.
plt.show()
if parameters.LSST_SAVEFIGPATH:
fig.savefig(os.path.join(parameters.LSST_SAVEFIGPATH, 'deconvolution_truth.pdf'), transparent=True)
plt.show()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@erykoff I assume this was deleted because there is a show only 3 lines above.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if it's DISPLAY it should show. This was showing the figure even if DISPLAY was false.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 35665721009

Coverage increased (+0.2%) to 89.012%

Details

  • Coverage increased (+0.2%) from the base build.
  • Patch coverage: 4 uncovered changes across 3 files (2 of 6 lines covered, 33.33%).
  • 2 coverage regressions across 1 file.

Uncovered Changes

File Changed Covered %
spectractor/extractor/background.py 2 0 0.0%
spectractor/extractor/images.py 2 1 50.0%
spectractor/extractor/spectrum.py 2 1 50.0%

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
spectractor/extractor/targets.py 2 80.44%

Coverage Stats

Coverage Status
Relevant Lines: 8418
Covered Lines: 7493
Line Coverage: 89.01%
Coverage Strength: 0.89 hits per line

💛 - Coveralls

@jeremyneveu jeremyneveu left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Happy with the changes! If the work is finished, let me know if I can merge.

@timj

timj commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor Author

@jeremyneveu Great. When you merge can you please then give us a formal release? It doesn't have to be on pypi. A GitHub release like you've done before is fine. Then we can make a conda-forge release.

There are no more patches coming from us.

@jeremyneveu
jeremyneveu merged commit 7803c1a into LSSTDESC:master Sep 22, 2026
2 checks passed
@jeremyneveu

Copy link
Copy Markdown
Collaborator

Here is the release version: https://github.com/LSSTDESC/Spectractor/releases/tag/v3.2.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants