Skip to content

BUG: read_excel failing to check older xlrd versions properly #39355

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

Merged
merged 12 commits into from
Feb 1, 2021

Conversation

lithomas1
Copy link
Member

Probably should be in 1.2.1 given that this occurred after the deprecation of the xlrd engine in 1.2.
cc @rhshadrach

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Thanks @lithomas1! Seems to me we should be using _optional._get_version. Also I think there is one in the tests - can you change that too.

@rhshadrach rhshadrach added IO Excel read_excel, to_excel Regression Functionality that used to work in a prior pandas version labels Jan 24, 2021
@rhshadrach rhshadrach added this to the 1.2.2 milestone Jan 24, 2021
@pep8speaks
Copy link

pep8speaks commented Jan 25, 2021

Hello @lithomas1! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-01-31 02:25:23 UTC

@rhshadrach
Copy link
Member

@lithomas1 I think we still want to use LooseVersion, can apply it to the result of _get_version.

@simonjayhawkins
Copy link
Member

pre-commit failing

pandas/io/excel/_base.py:5:1: F401 'distutils.version.LooseVersion' imported but unused
pandas/tests/io/excel/__init__.py:1:1: F401 'distutils.version.LooseVersion' imported but unused

@jreback
Copy link
Contributor

jreback commented Jan 28, 2021

@lithomas1 can you merge master and update

@rhshadrach
Copy link
Member

@lithomas1 Can also remove "_get_version" from scripts/validate_unwatned_patterns.py

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm - thanks @lithomas1!

@jreback jreback merged commit ddbf377 into pandas-dev:master Feb 1, 2021
@jreback
Copy link
Contributor

jreback commented Feb 1, 2021

thanks @lithomas1

@jreback
Copy link
Contributor

jreback commented Feb 1, 2021

@meeseeksdev backport 1.2.x

@lumberbot-app
Copy link

lumberbot-app bot commented Feb 1, 2021

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout 1.2.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 ddbf3771db13d927bb6d862e10c08b1c14e8d005
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #39355: BUG: read_excel failing to check older xlrd versions properly '
  1. Push to a named branch :
git push YOURFORK 1.2.x:auto-backport-of-pr-39355-on-1.2.x
  1. Create a PR against branch 1.2.x, I would have named this PR:

"Backport PR #39355 on branch 1.2.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

@jreback
Copy link
Contributor

jreback commented Feb 1, 2021

hmm this is not backporting cleanly, @lithomas1 can you follow the instructions above and put up a PR

simonjayhawkins pushed a commit to simonjayhawkins/pandas that referenced this pull request Feb 1, 2021
simonjayhawkins added a commit that referenced this pull request Feb 1, 2021
@lithomas1 lithomas1 deleted the xlrd-version-check branch February 1, 2021 17:38
@brobr
Copy link

brobr commented Dec 31, 2023

I ran into this bug on pandas-2.1

without xlrd present no problem; but when installed, excel file is not loaded:

In [5]: df_ref=pd.read_excel(refprots, engine="openpyxl", na_values =[''])
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
Cell In[5], line 1
----> 1 df_ref=pd.read_excel(refprots, engine="openpyxl", na_values =[''])

  ImportError: Can't determine version for xlrd

In [6]: df_ref=pd.read_excel(refprots, na_values =[''])
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
Cell In[6], line 1
----> 1 df_ref=pd.read_excel(refprots, na_values =[''])

  ImportError: Can't determine version for xlrd

In [7]: import xlrd

In [8]: xlrd.__version__
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[8], line 1
1 xlrd.__version__

AttributeError: module 'xlrd' has no attribute '__version__'

In [9]: xlrd.__VERSION__
Out[9]: '1.1.0'

In [10]: pd.__version__
Out[10]: '2.1.4'

@rhshadrach
Copy link
Member

@brobr - can you raise this as a new issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO Excel read_excel, to_excel Regression Functionality that used to work in a prior pandas version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: read_excel() fails when checking __version__ of older xlrd versions
6 participants