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

remove numba cache from fisher jenks #197

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Conversation

martinfleis
Copy link
Member

Numba is writing cache to tmp file in __pycache__ in the environment's site packages. If you install your conda env with admin rights and then try to run mapclassify with user rights, this will fail, as @jorisvandenbossche noticed while teaching.

The cache is there to cache JIT-ted version of the function so it does not have to be compiled in a new Python runtime again. There's a lot of gotcha's in the docs for caching, plus the one above. I suppose that the benefit of caching is in the end smaller than the issues it may cause, so I am suggesting removing cache=True.

@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #197 (6aeb3ae) into main (4417d31) will not change coverage.
The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #197   +/-   ##
=====================================
  Coverage   89.4%   89.4%           
=====================================
  Files          8       8           
  Lines       1088    1088           
=====================================
  Hits         973     973           
  Misses       115     115           
Files Coverage Δ
mapclassify/classifiers.py 88.4% <100.0%> (ø)

@jGaboardi
Copy link
Member

@martinfleis Good to merge?

@martinfleis
Copy link
Member Author

good from my side. so unless someone prefers to keep caching here, yes.

@jGaboardi
Copy link
Member

We can revert if someone has a strong opinion later.

@jGaboardi jGaboardi merged commit 4d82567 into pysal:main Oct 2, 2023
@martinfleis martinfleis deleted the cache branch October 2, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants