Adding support for writing BlobNode (imagery) #59
Conversation
Also including unit test validating support to fully copy an E57 structure
|
I had a look at the test file - it seems to be an unmodified copy of the standard e57 test file. After the xz hack attempt I was a bit nervous about accepting a pull request containing anonymous blob data 😁 |
tests/test_libe57.py
Outdated
| @pytest.fixture | ||
| def temp_e57_write(request): | ||
| path = sample_data("test_write.e57") | ||
| request.addfinalizer(lambda: delete_retry(path)) |
There was a problem hiding this comment.
issue: this function has not been imported
There was a problem hiding this comment.
My bad, last minute change before I pushed the code. 😅
tests/test_libe57.py
Outdated
| in_image = libe57.ImageFile(e57_path, "r") | ||
| out_image = libe57.ImageFile(temp_e57_write, "w") | ||
|
|
||
| for i in range(0, in_image.extensionsCount()): |
There was a problem hiding this comment.
nit: should be for i in range(in_image.extensionsCount()):
tests/test_libe57.py
Outdated
There was a problem hiding this comment.
question: could this sensibly be integrated with test_main.py? Some of the existing tests relate specifically to aspects of libe57 so it is confusing to split them in this way.
tests/test_libe57.py
Outdated
| compressed_node_pairs.extend(out_child_compressed_node_pairs) | ||
| blob_node_pairs.extend(out_child_blob_node_pairs) | ||
|
|
||
| if (isinstance(node, libe57.BlobNode)): |
There was a problem hiding this comment.
suggestion: change to elif
There was a problem hiding this comment.
suggestion: move above the container types with the other terminal nodes
tests/test_libe57.py
Outdated
|
|
||
| current_index += current_chunk | ||
|
|
||
| def copy_node(node, dest_image): |
There was a problem hiding this comment.
question: would it make sense to add this function to the main pye57 library? For instance in the utils namespace ?
tests/test_libe57.py
Outdated
|
|
||
| @pytest.fixture | ||
| def e57_path(): | ||
| # From http://www.libe57.org/data.html |
There was a problem hiding this comment.
praise: nice to have the source mentioned here
tests/test_libe57.py
Outdated
| @pytest.fixture | ||
| def temp_e57_write(request): | ||
| path = sample_data("test_write.e57") | ||
| request.addfinalizer(lambda: delete_retry(path)) |
There was a problem hiding this comment.
My bad, last minute change before I pushed the code. 😅
There was a problem hiding this comment.
Looks great - thanks again.
It would be very cool to add a tutorial or sample or just a reference to the clone e57 test case as an example of how to use the library to copy an e57 file. I also wonder whether the buffer functions would be good as part of utils but I will merge it for now as it stands !
When trying to clone an E57 file with images, I've realized a missing binding to support adding a BlobNode field to a StructureNode.
I've introduced a unit test file with an example of the clone process, in case you find it useful.