CharlieC (53497bf9) at 28 Mar 19:41
Make it possible to convert from CustomFilter to one of the Openpyx...
... and 6 more commits
The definition of whether a file conforms to the specification is not whether it can be opened by other applications. OOXML has a specification that is thousands of pages long and Openpyxl tries quite hard to follow it. It would be good to see other applications doing the same.
That file opens on Excel and Google Sheets, so I wouldn't say it's broken.
One thing I was thinking which might or might not be relevant or helpful, would be to add known extension namespaces to the registry. I can imagine them being useful in future implementations when we actively support extensions but handle them transparently, though I'm not sure how relevant this would be for the worksheet ones.
When we try to open a workbook that has an empty properties file (docProps/core.xml
), we get a lxml error for an empty document
>>> wb = load_workbook('uploads/test.xlsx')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/lib/python3.11/site-packages/openpyxl/reader/excel.py", line 346, in load_workbook
reader.read()
File "/usr/local/lib/python3.11/site-packages/openpyxl/reader/excel.py", line 293, in read
self.read_properties()
File "/usr/local/lib/python3.11/site-packages/openpyxl/reader/excel.py", line 175, in read_properties
src = fromstring(self.archive.read(ARC_CORE))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "src/lxml/etree.pyx", line 3257, in lxml.etree.fromstring
File "src/lxml/parser.pxi", line 1916, in lxml.etree._parseMemoryDocument
File "src/lxml/parser.pxi", line 1803, in lxml.etree._parseDoc
File "src/lxml/parser.pxi", line 1144, in lxml.etree._BaseParser._parseDoc
File "src/lxml/parser.pxi", line 618, in lxml.etree._ParserContext._handleParseResultDoc
File "src/lxml/parser.pxi", line 728, in lxml.etree._handleParseResult
File "src/lxml/parser.pxi", line 657, in lxml.etree._raiseParseError
File "<string>", line 1
lxml.etree.XMLSyntaxError: Document is empty, line 1, column 1
example file: test.xlsx
The workbook is broken. Please report the bug to whichever application or library created it.
Thanks for letting me know. You can check again against the 3.1 which has some improvements in this area, but the problem is indeed often down to other libraries being incorrect. It's worth noting that the timestamps in the core properties are UTC, although everywhere else they're local. I assume this the idea here is to make comparing versions a bit easier.
When we try to open a workbook that has an empty properties file (docProps/core.xml
), we get a lxml error for an empty document
>>> wb = load_workbook('uploads/test.xlsx')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/lib/python3.11/site-packages/openpyxl/reader/excel.py", line 346, in load_workbook
reader.read()
File "/usr/local/lib/python3.11/site-packages/openpyxl/reader/excel.py", line 293, in read
self.read_properties()
File "/usr/local/lib/python3.11/site-packages/openpyxl/reader/excel.py", line 175, in read_properties
src = fromstring(self.archive.read(ARC_CORE))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "src/lxml/etree.pyx", line 3257, in lxml.etree.fromstring
File "src/lxml/parser.pxi", line 1916, in lxml.etree._parseMemoryDocument
File "src/lxml/parser.pxi", line 1803, in lxml.etree._parseDoc
File "src/lxml/parser.pxi", line 1144, in lxml.etree._BaseParser._parseDoc
File "src/lxml/parser.pxi", line 618, in lxml.etree._ParserContext._handleParseResultDoc
File "src/lxml/parser.pxi", line 728, in lxml.etree._handleParseResult
File "src/lxml/parser.pxi", line 657, in lxml.etree._raiseParseError
File "<string>", line 1
lxml.etree.XMLSyntaxError: Document is empty, line 1, column 1
example file: test.xlsx
I faced a similar issue but by using linked sheets to Sharepoint in Python 3.11 / openpyxl 3.1.2. By opening the ZIP file to see the internal contents of the XLSX, I found corrupted contents in the formulas with linked sheets to Sharepoint by URL. Could my issue be related to this one?
I have had this same problem. I created an Excel spreadsheet with the SAS Excel libname engine. After that I post-process this spreadsheet with openpyxl
to inset an image in a worksheet. Sometimes my code will work and sometimes it does not.
Eventually I found out that SAS indeed inserts an invalid ISO 8601 date in the modified property in core.xml when the time is before 10 am:
<dcterms:modified xsi:type="dcterms:W3CDTF">2024-03-27T 9:21:42-04:00</dcterms:modified>
Notice that the time only uses 1 digit and includes a blank. When I create this Excel spreadsheet later on the day, after 10 am, I don't have the issue. I will open a bug report at SAS.
To be honest, I don't expect much direct interaction at this level but I found the simplifications useful myself, and really, when you can use a NestedSequence, the resulting code makes a lot more sense and is easier to work with.
We can and should rely on the low-level unit tests whenever possible and extend them only when needed. If this means your files are surviving the roundtrip then things are as expected and this doesn't need explicitly testing for. It's when things don't work that we know we don't have sufficient tests.
The only time I think people will ever work want to work with Pivots is when they're changing the range of the data they work on.
snippet (f71dfeac) at 27 Mar 11:25
Need to double slash for safety
I've added tests for most types now and removed what I could with NestedSequence. There's possibly more improvements to make but that was quite involved. For the most part the xml samples come from generated files. I can share them but given one of them is me attempting to every possible thing with a pivotCache it might be better suited for testing at a higher level than this? It results in a few pivotCacheDefinition xml files which could probably just be bundled up too.
snippet (af0f090d) at 27 Mar 11:19
Add a lot of tests, fixup issues found in testing
... and 1 more commit
Ah that would explain it. I'll have a bit of a closer look at the other members then and add some more tests
I think you're simply looking at some autogenerated stuff that I never got round to implementing or testing because I had no test cases for it. If I was, I'd do some renaming (who wants anything that sounds like PTSD?) and restructuring: can the children of a TupleCache be implemented using a NestedSequence?
The convention of plural->singular drove me mad which is why I use NestedSequences when I can or otherwise ComplexTypeList containing whatever it has to, but a NestedSequence is usually what's required.
Always start by stubbing out the missing tests with the generator for these stupid elements. Coverage is helpful here:
charlieclark:openpyxl/ $ pytest --cov openpyxl/pivot/ --cov-report term-missing openpyxl/pivot [18:22:18]
===================================================== test session starts =====================================================
platform darwin -- Python 3.8.19, pytest-8.1.1, pluggy-1.4.0
rootdir: /Users/charlieclark/Projects/openpyxl
configfile: pytest.ini
plugins: cov-4.1.0
collected 80 items
openpyxl/pivot/tests/test_cache.py .......................... [ 32%]
openpyxl/pivot/tests/test_fields.py .............. [ 50%]
openpyxl/pivot/tests/test_record.py ..... [ 56%]
openpyxl/pivot/tests/test_table.py ................x.................x [100%]
---------- coverage: platform darwin, python 3.8.19-final-0 ----------
Name Stmts Miss Cover Missing
----------------------------------------------------------
openpyxl/pivot/__init__.py 0 0 100%
openpyxl/pivot/cache.py 599 105 82% 152-155, 207-208, 224-225, 252-258, 272-273, 296-300, 322-326, 358-368, 382-383, 397-398, 422-427, 443-444, 468-473, 487-488, 500, 514-515, 627-632, 637, 653-654, 681-688, 710-714, 869-875, 887, 903, 908, 926-928, 1110-1119
openpyxl/pivot/fields.py 181 5 97% 38-40, 54-55
openpyxl/pivot/record.py 49 1 98% 111
openpyxl/pivot/table.py 707 49 93% 42, 58, 63, 79, 83, 160, 204-205, 209, 237-245, 285-298, 368, 687, 1232-1241, 1248-1252, 1261
----------------------------------------------------------
TOTAL 1536 160 90%
Please start with some unit tests to demonstrate what needs changing. Otherwise, it's very difficult to understand the details.
Why change the warning?
Why have you changed the behaviour of the class so much without providing unit tests?
Why has this module been deleted? These are valid OOXML objects but support hasn't been implemented.