-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: use internal linkage (static variables) in move.c #24113
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
BUG: use internal linkage (static variables) in move.c #24113
Conversation
move.c declared global variables without explicitly declaring them static, so they had global visibility. This meant that if you linked against a shared object that provided the same symbols, the wrong data would be read. In particular: linking to libgtk, which provides the symbol 'methods', would cause an import error of pandas.util._move.
Codecov Report
@@ Coverage Diff @@
## master #24113 +/- ##
==========================================
- Coverage 92.2% 92.2% -0.01%
==========================================
Files 162 162
Lines 51714 51714
==========================================
- Hits 47682 47681 -1
- Misses 4032 4033 +1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24113 +/- ##
==========================================
- Coverage 92.2% 92.2% -0.01%
==========================================
Files 162 162
Lines 51701 51701
==========================================
- Hits 47672 47671 -1
- Misses 4029 4030 +1
Continue to review full report at Codecov.
|
unclear how/if we want to test this case. Also, do you want a whatsnew for a (small) bugfix like this? |
thanks @llllllllll this looks fine. can you add a whatsnew note, ping on green. |
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.
minor point. ping on green.
doc/source/whatsnew/v0.24.0.rst
Outdated
@@ -1262,6 +1262,7 @@ Categorical | |||
- In meth:`Series.unstack`, specifying a ``fill_value`` not present in the categories now raises a ``TypeError`` rather than ignoring the ``fill_value`` (:issue:`23284`) | |||
- Bug when resampling :meth:`Dataframe.resample()` and aggregating on categorical data, the categorical dtype was getting lost. (:issue:`23227`) | |||
- Bug in many methods of the ``.str``-accessor, which always failed on calling the ``CategoricalIndex.str`` constructor (:issue:`23555`, :issue:`23556`) | |||
- Bug in :mod:`pandas.utils._move` where C variables were declared with external linkage causing import errors if certain other C libraries were imported before Pandas. (:issue:`24113`) |
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.
can you move to the other section, and remove the :mod:`pandas.utils._move`
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.
just remove the link, or don't mention which module the error was in?
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.
just remove the module, i think the statement about c libraries is ok (but move it to the 'other' section, at the bottom)
db17d85
to
8b62a41
Compare
I moved the whatsnew entry to other and removed the link. Sorry about putting it in "categorical" before, I just searched for "bugs" quickly. Tests are now passing. cc @jreback |
Thanks! |
) * BUG: use internal linkage (static variables) in move.c move.c declared global variables without explicitly declaring them static, so they had global visibility. This meant that if you linked against a shared object that provided the same symbols, the wrong data would be read. In particular: linking to libgtk, which provides the symbol 'methods', would cause an import error of pandas.util._move. * DOC: whatsnew entry for pandas.util._move static variables
) * BUG: use internal linkage (static variables) in move.c move.c declared global variables without explicitly declaring them static, so they had global visibility. This meant that if you linked against a shared object that provided the same symbols, the wrong data would be read. In particular: linking to libgtk, which provides the symbol 'methods', would cause an import error of pandas.util._move. * DOC: whatsnew entry for pandas.util._move static variables
move.c declared global variables without explicitly declaring them static, so
they had global visibility. This meant that if you linked against a shared
object that provided the same symbols, the wrong data would be read. In
particular: linking to libgtk, which provides the symbol 'methods', would cause
an import error of pandas.util._move.
git diff upstream/master -u -- "*.py" | flake8 --diff