Initial TDSX support. #44
Conversation
| @@ -0,0 +1,67 @@ | |||
| import contextlib | |||
There was a problem hiding this comment.
'archivefile' is obviously a terrible name
There was a problem hiding this comment.
hah, containerfile? It's only a zip archive because of implementation details. Ultimately, it's a container file format.
(also, I'm horrible at naming things)
There was a problem hiding this comment.
containerfile works for me.
<Insert joke about naming things is hard here / >
There was a problem hiding this comment.
what exactly are we trying to convey here? just that there is some kind of container (i.e. TDSX or TWBX) vs. a single file (TDS or TWB)?
if these containers will always end in "x" then let's call this xfile.py :)
There was a problem hiding this comment.
Oh damn, @benlower with the winning name! we should go with xfiles
… from Workbook class
…pen the file directly from the zip instead of extracting multiple times
| xml_file = find_file_in_zip(zf, file_type) | ||
| xml_tree = ET.parse(os.path.join(temp, xml_file)) | ||
| with zipfile.ZipFile(filename) as zf: | ||
| xml_file = zf.open(find_file_in_zip(zf)) |
There was a problem hiding this comment.
Is this a context object also? Or does it automatically get closed when the zipfile is closed?
There was a problem hiding this comment.
You're right, it needs to be wrapped in a with
|
one small comment about the zt.open usage, but otherwise LGTM. |
|
@benlower @RussTheAerialist alright, the code is feeling much better -- but we have one last chance to call that module something else before merging. Do we want to go with xfile :) ? |
Lots of duplicated code with TWBX's. But it works.
So much so, I wouldn't really want to check this in as-is.
I have a vague idea of an ArchiveFile module to hold all the shared logic, or something along those lines.
Thoughts? @RussTheAerialist