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

BUG: use internal linkage (static variables) in move.c #24113

Merged

Conversation

llllllllll
Copy link
Contributor

@llllllllll llllllllll commented Dec 5, 2018

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.

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
Copy link

codecov bot commented Dec 5, 2018

Codecov Report

Merging #24113 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            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
Flag Coverage Δ
#multiple 90.6% <ø> (ø) ⬆️
#single 43.01% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/io/json/json.py 92.61% <0%> (-0.48%) ⬇️
pandas/util/testing.py 87.51% <0%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f2c716...c88d9ea. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 5, 2018

Codecov Report

Merging #24113 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            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
Flag Coverage Δ
#multiple 90.6% <ø> (ø) ⬆️
#single 43.02% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/io/json/json.py 92.61% <0%> (-0.48%) ⬇️
pandas/util/testing.py 87.51% <0%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b78aa8d...8b62a41. Read the comment docs.

@llllllllll
Copy link
Contributor Author

unclear how/if we want to test this case. Also, do you want a whatsnew for a (small) bugfix like this?

@jreback
Copy link
Contributor

jreback commented Dec 5, 2018

thanks @llllllllll this looks fine. can you add a whatsnew note, ping on green.

@jreback jreback added Bug Compat pandas objects compatability with Numpy or Python functions labels Dec 5, 2018
@jreback jreback added this to the 0.24.0 milestone Dec 5, 2018
Copy link
Contributor

@jreback jreback left a 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.

@@ -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`)
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 to the other section, and remove the :mod:`pandas.utils._move`

Copy link
Contributor Author

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?

Copy link
Contributor

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)

@llllllllll
Copy link
Contributor Author

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

@TomAugspurger TomAugspurger merged commit 03134cb into pandas-dev:master Dec 6, 2018
@TomAugspurger
Copy link
Contributor

Thanks!

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
)

* 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
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
)

* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Compat pandas objects compatability with Numpy or Python functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to import pandas when using the gtk3 backend in ipython
3 participants