Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smoke tests failing on new SRD image #1759

Closed
5 tasks done
craddm opened this issue Mar 15, 2024 · 7 comments
Closed
5 tasks done

Smoke tests failing on new SRD image #1759

craddm opened this issue Mar 15, 2024 · 7 comments
Labels
bug Problem when deploying a Data Safe Haven.
Milestone

Comments

@craddm
Copy link
Contributor

craddm commented Mar 15, 2024

✅ Checklist

  • I have searched open and closed issues for duplicates.
  • This is a problem observed when deploying a Data Safe Haven.
  • I can reproduce this with the latest version.
  • I have read through the documentation.
  • This isn't an open-ended question (open a discussion if it is).

💻 System information

  • Operating System: macOS
  • Data Safe Haven version: v4.2.0

🌵 Powershell module versions

N/A

🚫 Describe the problem

Smoke tests fail for importing Python packages. There is a missing dependency, dask-expr, which cause lightgbm to fail. Possibly lightgbm does not have dask-expr as a dependency, only some submodules of dask, but uses dask functionality that now requires dask-expr as a separate package.

🌳 Log messages

Screenshot 2024-03-15 at 16 54 06

Note that the same error occurs for all Python versions, not just 3.12.2

♻️ To reproduce

Run smoke tests on the most recent SRD image.

@craddm craddm added the bug Problem when deploying a Data Safe Haven. label Mar 15, 2024
@craddm craddm added this to the v4.2.0 milestone Mar 15, 2024
@craddm craddm mentioned this issue Mar 15, 2024
9 tasks
@JimMadge
Copy link
Member

From the PyPI page

To install all dependencies needed to use lightgbm.dask, append [dask].

pip install 'lightgbm[dask]'

@JimMadge
Copy link
Member

Is this a dependency for another package?

@craddm
Copy link
Contributor Author

craddm commented Mar 20, 2024

Is this a dependency for another package?

I'm not quite sure what you mean. I don't know if another package has dask as a dependency, but I do know what I wrote above was confusingly worded. I suspect that the requirement for dask-expr is not getting picked up by the dependency solver.

We do already have dask in our packages list, and this would have covered lightgbm's dependency on it until the most recent release of dask. The issue is that in the most recent release of dask, the default backend for dask.dataframe changed to dask-expr. That seems to need to be installed separately, although it does get installed if you install lightgbm[dask]. Simplest thing to do is add dask-expr to our Python list.

@JimMadge
Copy link
Member

I mean, do we install lightgbm because we have explicitly included it, or because it is a dependency of another package with explicitly install?

Reading the docs for lightgbm, I get the impression they expect you to use the [dask] extra to get the requirements needed to work with dask. It seems odd that you can't import lightgbm though, extra tend to be used for optional dependencies.

I think it would be better to install lightgbm[dask] rather than add another package to the install list.

@craddm
Copy link
Contributor Author

craddm commented Mar 21, 2024

I've switched to using lightgbm[dask] now. It seems the error on import is a known bug introduced by the latest version of dask - see microsoft/LightGBM#6365 and dask/dask#11007

@JimMadge
Copy link
Member

And lightgbm[dask] fixes the problem because of the extra dependencies it installs?

@craddm
Copy link
Contributor Author

craddm commented Mar 25, 2024

Yes, this one is fixed now

@JimMadge JimMadge mentioned this issue Mar 25, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem when deploying a Data Safe Haven.
Projects
None yet
Development

No branches or pull requests

2 participants