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

Task/weak numba dependency #26

Merged
merged 33 commits into from
Nov 20, 2024
Merged

Task/weak numba dependency #26

merged 33 commits into from
Nov 20, 2024

Conversation

johannvk
Copy link
Contributor

This PR adds numba as a weak / optional dependency of skchange, instead of the hard dependency it is now.
To achieve this, we use the sktime function _check_soft_dependencies, and return a passthrough version of the @jit and @njit decorators if numba is not installed.

Additionally, this PR enables the configuring of some numba default @njit parameters through the setting of environment variables.
Currently the supported parameters are cache, parallel, and fastmath, set by the corresponding environment variables NUMBA_CACHE, NUMBA_PARALLEL, and NUMBA_FASTMATH.
The current implementation only sets the defaults, so if a user of the numba decorator specifies e.g. fastmath=False, their choice will not be overridden.

Copy link

codecov bot commented Nov 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.98%. Comparing base (b66f3f4) to head (1e3f6fe).
Report is 34 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
- Coverage   97.06%   96.98%   -0.08%     
==========================================
  Files          44       33      -11     
  Lines        1873     1395     -478     
==========================================
- Hits         1818     1353     -465     
+ Misses         55       42      -13     

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


🚨 Try these New Features:

pyproject.toml Show resolved Hide resolved
skchange/utils/numba/njit.py Outdated Show resolved Hide resolved
skchange/utils/numba/njit.py Outdated Show resolved Hide resolved
@Tveten
Copy link
Collaborator

Tveten commented Nov 15, 2024

Another thing: I see the patch coverage is pretty low. Do you have time to write a few tests for the new njit module @johannvk? They can be placed in skchange/utils/numba/tests

@johannvk
Copy link
Contributor Author

Another thing: I see the patch coverage is pretty low. Do you have time to write a few tests for the new njit module @johannvk? They can be placed in skchange/utils/numba/tests

It took some trial and error, but I added tests for all the new configuration fuctionality now, and also added another test step to the Github Actions workflow to run all our tests with and without Numba jit-compilation.

@Tveten Tveten merged commit 1e6a214 into main Nov 20, 2024
5 of 6 checks passed
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