Skip to content

Add romanisim package to devdeps - #2184

Merged
schlafly merged 3 commits into
mainfrom
add-romanisim-to-devdeps
Sep 23, 2026
Merged

schlafly merged 3 commits into
mainfrom
add-romanisim-to-devdeps

Conversation

@schlafly

Copy link
Copy Markdown
Collaborator

We don't currently test against romanisim main in our devdeps, but we probably should? This PR adds romanisim to the list of devdeps we test nightly against.

@schlafly
schlafly requested a review from a team as a code owner February 12, 2026 21:57
@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label Feb 12, 2026
@codecov

codecov Bot commented Feb 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.06%. Comparing base (3b88606) to head (14b1f2b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2184   +/-   ##
=======================================
  Coverage   84.06%   84.06%           
=======================================
  Files         132      132           
  Lines        7419     7419           
=======================================
  Hits         6237     6237           
  Misses       1182     1182           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@braingram braingram 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.

LGTM. This will require updating #1914 Any opinions on that PR?

@schlafly

Copy link
Copy Markdown
Collaborator Author

Sorry I had lost track of #1914, approved.

@schlafly
schlafly enabled auto-merge (squash) February 13, 2026 14:02
@braingram
braingram force-pushed the add-romanisim-to-devdeps branch from f430d10 to bd568f1 Compare February 19, 2026 17:46
@braingram
braingram disabled auto-merge February 19, 2026 17:56
@braingram

Copy link
Copy Markdown
Collaborator

Since this is a devdeps change I added the extra tests label and disabled automerge (to avoid merging before the devdeps jobs finish).

@braingram

Copy link
Copy Markdown
Collaborator

devdeps picked up the changes:
https://github.com/spacetelescope/romancal/actions/runs/22067448092/job/63762945639?pr=1914#step:10:36
The failures are unrelated (fixed in #2188)
I'm going to re-enable automerge since the tests that have finished sufficiently tested this change.

@braingram

Copy link
Copy Markdown
Collaborator

@ddavis-stsci would you give this a review since it's merging is blocked by needing maintainer approval?

@zacharyburnett
zacharyburnett force-pushed the add-romanisim-to-devdeps branch from bd568f1 to a0af635 Compare February 19, 2026 19:36
@braingram
braingram force-pushed the add-romanisim-to-devdeps branch from a0af635 to e74e57f Compare February 19, 2026 19:36
@zacharyburnett

zacharyburnett commented Feb 19, 2026

Copy link
Copy Markdown
Collaborator

py313-devdeps failed with the following environment resolution conflict:
https://github.com/spacetelescope/romancal/actions/runs/22197066133/job/64200100951?pr=2184#step:10:204

The conflict is caused by:
    The user requested roman-datamodels 0.31.0.dev1+g94d2f8d3b (from git+https://github.com/spacetelescope/roman_datamodels.git)
    romanisim 0.13.1.dev3+g27be099b4 depends on roman_datamodels<0.31.0 and >=0.29.0

Unfortunately, since romanisim and romancal both require roman_datamodels, the only time the -devdeps install WON'T break is when both pin to a PyPI release (instead of a Git URL)

@braingram

Copy link
Copy Markdown
Collaborator

Why did this work prior to #1914 being merged?

@zacharyburnett

Copy link
Copy Markdown
Collaborator

Oh of course, you're absolutely right

@braingram
braingram marked this pull request as draft February 19, 2026 20:03
auto-merge was automatically disabled February 19, 2026 20:03

Pull request was converted to draft

@braingram

Copy link
Copy Markdown
Collaborator

I moved this back to draft since it's no longer working.

@zacharyburnett

Copy link
Copy Markdown
Collaborator

looks like that worked

@braingram
braingram force-pushed the add-romanisim-to-devdeps branch from e0f370e to 8c34150 Compare February 24, 2026 15:54
@braingram
braingram marked this pull request as ready for review February 24, 2026 15:59
@zacharyburnett
zacharyburnett force-pushed the add-romanisim-to-devdeps branch from 8c34150 to 5313e40 Compare February 25, 2026 14:26
@schlafly

Copy link
Copy Markdown
Collaborator Author

I was trying to come back and clean up some of these old PRs and took another look at this one. @zacharyburnett , if you get a chance, could you see if the devdeps errors make sense to you?

@zacharyburnett
zacharyburnett force-pushed the add-romanisim-to-devdeps branch from 7579ef1 to 9d75664 Compare September 14, 2026 13:23
@zacharyburnett

zacharyburnett commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

So the problem here is we're requesting rad@main, however roman_datamodels@main currently constrains rad>=1.1.0,<1.2.0, which are released versions on PyPI:

  × No solution found when resolving dependencies:
  ╰─▶ Because only roman-datamodels==1.2.0.dev5+g224241f3a is available and
      roman-datamodels==1.2.0.dev5+g224241f3a depends on rad>=1.1.0,<1.2.0,
      we can conclude that all versions of roman-datamodels depend on
      rad>=1.1.0,<1.2.0.

I'm unsure how to make this work with the current version of tox; my go to solution would have been just doing a pip install -r requirements-dev.txt in commands_pre, which occurs after the install step so would override everything without complaint; HOWEVER, testing has revealed that even running pip in commands_pre doesn't work anymore:

py3-devdeps-xdist: commands_pre[1]> pip install -r requirements-dev.txt
py3-devdeps-xdist: Exception running subprocess [Errno 2] No such file or directory: 'pip'

This also means that now our existing oldestdeps toxenv config won't work anymore either:

py3-oldestdeps-xdist: commands_pre[1]> pip install -r requirements-min.txt
py3-oldestdeps-xdist: Exception running subprocess [Errno 2] No such file or directory: 'pip'

@zacharyburnett

zacharyburnett commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

I'll see if an older version of tox restores this functionality. In the meantime, I'll also explore how much work it might be to convert our tox.ini to a noxfile.py and use Nox instead (it's reportedly designed to be easy to migrate from Tox)

@braingram braingram 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.

Looks like it's not pulling in main from the version (0.14.0):
https://github.com/spacetelescope/romancal/actions/runs/35661144536/job/106536484534?pr=2184#step:11:116
@schlafly the pin in pyproject.toml doesn't allow 0.15.0:

"romanisim>=0.14,<0.15",

which appears to be the latest release:
https://github.com/spacetelescope/romanisim/releases/tag/v0.15.0
Does something need updating?

@schlafly

Copy link
Copy Markdown
Collaborator Author

Thanks, I'm trying to loosen the pin here: #2463

@schlafly

Copy link
Copy Markdown
Collaborator Author

This finally looks good. It was useful; I had broken romancal in spacetelescope/romanisim#379, so I fixed romanisim in spacetelescope/romanisim#396, fixing the issue. That had the side effect of verifying that this PR is now functional. I'm ready to merge this.

@schlafly
schlafly requested a review from braingram September 23, 2026 21:14

@braingram braingram 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.

@schlafly
schlafly merged commit d8dc802 into main Sep 23, 2026
36 of 38 checks passed
@schlafly
schlafly deleted the add-romanisim-to-devdeps branch September 23, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants