8000 Accept constant memoryviews in HashTable.lookup by xhochy · Pull Request #21688 · pandas-dev/pandas · GitHub
[go: up one dir, main page]
Skip to content

Accept constant memoryviews in HashTable.lookup #21688

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Jul 7, 2018
Merged

Accept constant memoryviews in HashTable.lookup #21688

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
475e99d
Accept constant memoryviews in HashTable.lookup
xhochy Jun 29, 2018
bcd1384
Add const specifiers to remaining signatures
xhochy Jun 30, 2018
081067c
Increase minimal Cython version to 0.28
xhochy Jun 30, 2018
e42a718
Add whatsnew section
xhochy Jun 30, 2018
bf4eed7
Add tests for readonly memoryviews
xhochy Jun 30, 2018
6c9341b
Bump Cython version in CI to 0.28
xhochy Jul 1, 2018
6c335b0
Apply modern fix for #18825
xhochy Jul 7, 2018
9deaf07
Start new section 'Build Changes' in whatsnew
xhochy Jul 7, 2018
e0b7098
Test writable arrays with parameters
xhochy Jul 7, 2018
20c181f
Bump CI version for Cython to 0.28.2
xhochy Jul 7, 2018
0552d61
add writeable fixture & fix docs
jreback Jul 7, 2018
64fd445
bump min version
jreback Jul 7, 2018
a4cb629
add min cython versions everywhere
jreback Jul 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Start new section 'Build Changes' in whatsnew
  • Loading branch information
@xhochy
xhochy committed Jul 7, 2018
commit 9deaf07112f3951be41d167af14ee5d4543ca2b1
5 changes: 4 additions & 1 deletion doc/source/whatsnew/v0.24.0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,16 @@ Reshaping
-
-

Build Changes
^^^^^^^^^^^^^

- Building pandas for development now requires ``cython >= 0.28`` (:issue:`21688`)
-

Other
^^^^^

- :meth: `~pandas.io.formats.style.Styler.background_gradient` now takes a ``text_color_threshold`` parameter to automatically lighten the text color based on the luminance of the background color. This improves readability with dark background colors without the need to limit the background colormap range. (:issue:`21258`)
- Building pandas for development now requires ``cython >= 0.28``
- Require at least 0.28 version of cython to support read-only memoryviews (:issue:`21688`)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referenced the PR as issue here. Is that sufficient or would you like to have separate issues for each line for documentation purposes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we really need to bump cython here?

Copy link
Member
@jschendel jschendel Jul 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming we need to bump Cython, it looks like there are a few things missing in order to fully bump it: pin some CI builds to the minimum Cython version, update version number in install.rst.

See the changes in #18623 for where I bumped to 0.24; should show all the places that need updating.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading through the changelog of Cython, we need to bump to 0.28 to support the const specifier for memoryviews. Looking a bit through the Travis setup, we only pin Cython for the 2.7 builds and these are the ones that failed. (sadly with a weird error message).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to bump to 0.28 to support the const specifier for memoryviews

is this an actual change in cython?

I agree its nice to change this, but cython 0.28 is pretty new.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move the building pandas to a new sub-section (like we did previously when we bumped deps, as going to do this for more deps this cycle).

also is 0.28 min, or 0.28.2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.28.0 works but we should build with 0.28.2 due to the mentioned problems in sklearn. I would at least bump the Cython version on Travis & CricleCI.

Should we then allow 0.28.0 or require 0.28.2 so that we don't get the bug reports w.r.t. problems that are fixes in .2?

-
-
Expand Down
0