Skip to content

Read cell comments in .xlsx files #58

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 6 commits into from
Jan 25, 2014
Merged

Read cell comments in .xlsx files #58

merged 6 commits into from
Jan 25, 2014

Conversation

stephenlewis
Copy link
Contributor

This builds on the work done in #38.

It adds tests for parsing comments out of .xlsx files generated by Excel 2007 and Google Docs, and copes correctly with the two different comment structures identified in the previous pull request.

gthb and others added 3 commits March 5, 2013 22:20
Works for the single file I've tried it on, for what that's worth. Could
do with a bit more testing.

Fixes python-excel/xlrd#37
This has test cases for .xlsx files generated by both Excel 2007 and
Google Docs, and works for 't' elements nested directly under 'text' or
with an intervening 'r' element.

It also copes with the (common) Excel case where there are multiple 't'
elements: this occurs when a comment has an emboldened author name
@stephenlewis
Copy link
Contributor Author

@gthb @sjmachin Python is not a language I'm particularly familiar with, I'm afraid, and I can't understand the test failure under 2.6 - are you able to help (even if it's a matter of pointing me at docs)? Thanks!

@stephenlewis
Copy link
Contributor Author

@gthb @sjmachin Avoiding the use of '//' when using ET.findall seems to have done the trick. That said, Python's not my usual language, so please do suggest improvements if any spring to mind.

@sjmachin
Copy link
Member

sjmachin commented Jul 2, 2013

Your diagnosis of the 2.6 problem appears to be correct.

One thing to fix: With the Excel UI it is quite possible to create a comment which contains no text. This is pointless but still valid. Please drop the warning, and create a Note object with text = a zero-length string i.e. not None.

@stephenlewis
Copy link
Contributor Author

Good catch - thanks. I don't have access to a copy of Excel at present, but I'll add another test with code to deal with that case tomorrow.

@eirnym
Copy link

eirnym commented Nov 27, 2013

I've tested this patch with price files from Bitrix CMS and it seems OK for me. My environment is Mac OS X, Python 3.3.3

@cjw296
Copy link
Member

cjw296 commented Jan 18, 2014

@stephenlewis - still waiting for you test to deal with the case @sjmachin raises...

@stephenlewis
Copy link
Contributor Author

@cjw296 I've just pushed a commit that addresses @sjmachin 's comment - thanks for the reminder.

cjw296 added a commit that referenced this pull request Jan 25, 2014
@cjw296 cjw296 merged commit 92be493 into python-excel:master Jan 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants