Skip to content

make gpax and sable optional dependencies - #51

Merged
konstpieper merged 4 commits into
update_fixfrom
use-optional-dependencies
Jul 24, 2026
Merged

make gpax and sable optional dependencies#51
konstpieper merged 4 commits into
update_fixfrom
use-optional-dependencies

Conversation

@Lance-Drane

@Lance-Drane Lance-Drane commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

We can immediately merge #49 once this is merged into the update_fix branch.

Keeping scikit-learn as a required dependency for now to avoid breaking backwards compatibility, as it takes up very little space on its own (most of the space is taken up by scipy, which we want to have as a required dependency). This can be changed if desired.

Since Sable pulls in so many dependencies but is not necessarily a requirement for using DIAL, it is being moved to optional dependencies.

I have also moved gpax to be an optional dependency, the size is not really a problem here either but this is more due to it not receiving first class support + reportedly causing some people build issues (though I think these have been resolved).

Closes #46

Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
@Lance-Drane
Lance-Drane requested a review from konstpieper July 24, 2026 01:37
@konstpieper

Copy link
Copy Markdown
Contributor

I think this is fine, but it will break unit tests and benchmarks depending on those backends. Do we have a mechanism to disable tests/benchmarks that use a specific backend?

Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
@Lance-Drane

Lance-Drane commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

I think this is fine, but it will break unit tests and benchmarks depending on those backends. Do we have a mechanism to disable tests/benchmarks that use a specific backend?

Good catch, fixed in latest commit. You can use pytest.param inside the list of parameters, and decorate that parameter with pytest.mark.skipif (it's easiest to just piggyback off of the AVAILABLE_DIAL_BACKENDS variable, because we already handle the "do we have the backend installed" logic in the Service)

Examples: https://docs.pytest.org/en/stable/how-to/skipping.html#skip-xfail-with-parametrize

This should work extremely well for unit tests. For benchmarks (where we reuse the parameters in contexts outside of the pytest statement), this is a little awkward because we really need to have a backend parameter. I decided that the least obnoxious implementation would be to provide a helper function to generate the pytest marks from the parameters, where all you need to provide is the index of your backend argument. This is obtuse, but less so than requiring the parameter to be called backend or other strategies.

Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
@konstpieper
konstpieper merged commit bc8ec52 into update_fix Jul 24, 2026
@Lance-Drane
Lance-Drane deleted the use-optional-dependencies branch July 28, 2026 00:29
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.

2 participants