Remove fsps from requirements - #347
Conversation
|
I added the |
bsipocz
left a comment
There was a problem hiding this comment.
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)
|
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. |
032abda to
1cb8b2a
Compare
Build is still failing, need to investigate more closely -- the logs of past builds are not very useful for this :(
1cb8b2a to
221c003
Compare
221c003 to
cdaeae7
Compare
bsipocz
left a comment
There was a problem hiding this comment.
I would think we could add a test job that opts in the SED fitting, but that should be separate from this PR.
|
For future reference:
We could work around that, but we discovered that the OpenUniverse SED notebook doesn't use |
|
Upstream report: dfm/python-fsps#388 (but I think we should stick with python 3.12 for the fsps enabled case) |
|
Ah, thanks for reporting upstream. Realized I didn't do it while trying to fall asleep last night. |
Closes #346