Skip to content

Remove fsps from requirements - #347

Merged
troyraen merged 2 commits into
Caltech-IPAC:mainfrom
troyraen:issues/346/scikit-build-core
Jul 7, 2026
Merged

Remove fsps from requirements#347
troyraen merged 2 commits into
Caltech-IPAC:mainfrom
troyraen:issues/346/scikit-build-core

Conversation

@troyraen

@troyraen troyraen commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Closes #346

@troyraen troyraen added infrastructure Infrastructure related issues/PRs. blocks deploy The affected notebook(s) cause cron job failures and/or block(s) HTML and Fornax deployment. GHA buildhtml Enable extra buildhtml job on GHA labels Jul 6, 2026
@troyraen
troyraen requested a review from bsipocz July 6, 2026 22:57
@troyraen troyraen added GHA buildhtml Enable extra buildhtml job on GHA and removed GHA buildhtml Enable extra buildhtml job on GHA labels Jul 6, 2026
@troyraen

troyraen commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

I added the GHA buildhtml label to check whether notebooks still run but that job isn't executing. Tried triggering it manually after adding the label but still no luck. Does it require pushing a new commit after the label has been applied?

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should instead:

  • limit the scikit-build version number as we'll run into this very same problem again when we enable fsps OR
  • remove all the new dependencies brought in by that notebook -- so it's all consistent

At minimum we should maybe consider commenting the dependency out with a note that it's temporary and is due to scikit-build. Bottom line, it's OK to do a quick patch to fix up seemingly unrelated failures, but we should make our life easier to reverse it back when the workaround is expected to be needed for a longer period of time (e.g. I don't expect fsps will be new scikit-build compatible any time soon, if ever)

@troyraen

troyraen commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, I thought this notebook was never executed based on #315 and that fsps was just left in the requirements file by mistake. Looking closer at the CI config files I see that it is executed in some cases. I'll pin scikit-build-core.

@troyraen
troyraen force-pushed the issues/346/scikit-build-core branch 2 times, most recently from 032abda to 1cb8b2a Compare July 7, 2026 18:35
bsipocz
bsipocz previously approved these changes Jul 7, 2026
@bsipocz
bsipocz dismissed their stale review July 7, 2026 18:54

Build is still failing, need to investigate more closely -- the logs of past builds are not very useful for this :(

@troyraen
troyraen force-pushed the issues/346/scikit-build-core branch from 1cb8b2a to 221c003 Compare July 7, 2026 20:09
@troyraen
troyraen force-pushed the issues/346/scikit-build-core branch from 221c003 to cdaeae7 Compare July 7, 2026 20:32

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would think we could add a test job that opts in the SED fitting, but that should be separate from this PR.

@troyraen

troyraen commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

For future reference:

fsps doesn't provide a wheel for python 3.13, so we end up building it from source. This ignores our requirements file and, since their toml doesn't pin scikit-build-core, it picks up latest which fails as of yesterday's scikit-build-core v1.0.0 release.

We could work around that, but we discovered that the OpenUniverse SED notebook doesn't use fsps by default (requires user to change RUN_FITS to True), so there's no need for our testing and deployment infrastructure to install it.

@troyraen
troyraen merged commit 5555e7f into Caltech-IPAC:main Jul 7, 2026
8 checks passed
@troyraen
troyraen deleted the issues/346/scikit-build-core branch July 7, 2026 21:58
@bsipocz

bsipocz commented Jul 8, 2026

Copy link
Copy Markdown
Member

Upstream report: dfm/python-fsps#388

(but I think we should stick with python 3.12 for the fsps enabled case)

@troyraen

troyraen commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Ah, thanks for reporting upstream. Realized I didn't do it while trying to fall asleep last night.

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

Labels

blocks deploy The affected notebook(s) cause cron job failures and/or block(s) HTML and Fornax deployment. GHA buildhtml Enable extra buildhtml job on GHA infrastructure Infrastructure related issues/PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scikit-build-core causing latest-deps job to fail

2 participants