-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add type signatures to jenksy #119
Conversation
… add a warning if numba is not installed. (pysal#118)
Two of the |
(sorry for closing and reopening, I wanted to make sure CI kicks in) |
Sure. No problem! The tests in the CI are passing. However, I used Python 3.10 and latest versions of the dependencies to run |
I used [tox]
envlist =
{py36,py37,py38}-{speedup,slow}
[testenv]
conda_deps=
pytest
pytest-cov
codecov
geopandas
libpysal
networkx
palettable
speedup: numba
commands=
pytest {posargs} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR looks okay to me! Thanks!
However, I used Python 3.10 and latest versions of the dependencies to run pytest on my machine and two of the greedy tests failed.
You can ignore that. I'll make a PR slimming down our CI matrix and including python 3.10 and look into this.
I am not sure why three of the CI's were stuck for 3 hours.
don't know either
|
Oh. So the issue has been there for a while. Sounds good! Let me know if this PR needs any other changes. |
It's been a while since I opened this PR, do I need to change anything in this PR? |
Per the discussion in #118, this PR does the following:
_fisher_jenks_means
sort
argument from_fisher_jenks_means
since it's not being passed from the class function and is alwaysTrue
.extra_req
tosetup.py
for installingnumba
asspeedups
.__pycach__
to.gitignore
sincenumba
compiles the function and keep in the cache folder.numba
is not installed the (slow) pure Python code is used.