-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
CI: testing wip branch on ci. #40530
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
Conversation
Hello @simonjayhawkins! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2021-03-21 13:11:55 UTC |
So looks like I can't put off dealing with the numba warnings in order to reduce the noise in the logs. |
asv_bench/benchmarks/arithmetic.py
Outdated
# warm-up for numba | ||
|
||
self.s2.diff() | ||
|
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.
I need to find a better solution that this. have now removed the changes to the benchmarks.
na = iNaT | ||
dtype = np.dtype("timedelta64[ns]") | ||
arr = getattr(arr, "_data", arr) | ||
na = None |
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.
was originally considering a feasibility of a drop in replacement for pandas._libs. Because numba does not require some of the conversions needed for the cython, it then made sense, to keep the jitted python code simpler. Changing tack again to original plan to limit scope to a drop-in replacement. The advantage is that should be simpler to keep in sync with pandas master. Taking full advantage of numba is therefore deferred for now.
libalgos.take_1d_int64_int64, np.int64, np.int64, np.int64 | ||
), | ||
} | ||
|
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.
have a commit ready to push that reverts this change to the main code base.. the jitted python code is so messy, especially trying to add specializations so that could match the cython signatures. The upside is that take_1d is ideal parallel candidate. Long term, if paralellization is performed in a higher level loop, i.e. columns in a DataFrame, then take would be serialized, but for now with the scope limited to a libs drop-in, then will parallelize at the libs level to keep the main codebase unchanged.
@@ -533,7 +534,7 @@ def astype(self, dtype: Dtype, copy: bool = True) -> ArrayLike: | |||
# error: Incompatible types in assignment (expression has type "ndarray", | |||
# variable has type "Categorical") | |||
result = take_nd( # type: ignore[assignment] | |||
new_cats, libalgos.ensure_platform_int(self._codes) | |||
new_cats, ensure_platform_int(self._codes) |
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.
will open a PR for this simple change
@@ -162,8 +162,7 @@ def test_transform_method_name(method): | |||
frame_kernels_raise = [x for x in frame_transform_kernels if x not in wont_fail] | |||
|
|||
|
|||
# mypy doesn't allow adding lists of different types | |||
# https://github.com/python/mypy/issues/5492 | |||
@pytest.mark.xfail(strict=False) |
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 xfails were because of tests failing due to additional warnings. will remove these and deal with the warnings.
have added numba to all the envs for testing purposes, so have reopened to re-run tests. should have now removed the warnings for take_1d in object mode. _pad_2d_inplace still has the object related warnings |
No need to review. will be closing to clear the queue later today.