Refactor OriginalLabel class to remove inheritance from Blob #4309
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jessemapel
reviewed
Feb 23, 2021
acpaquette
reviewed
Feb 23, 2021
Comment on lines
+938
to
+940
| void Cube::write(OriginalLabel lab) { | ||
| write(*(lab.toBlob())); | ||
| } |
There was a problem hiding this comment.
I know this write method makes things easier but I don't know how necessary it is and that's something I have been going back and forth on. It'd be good for us to figure out exactly how we want to be writing these blobs back out. While the *(lab.toBlob()) is a little messy I don't think it's bad to have write(*(lab.toBlob())) all over the place rather than write(lab)
|
For reading/writing these things, I think the purest thing to do is enforce File <-> Blob <-> Class. It requires a bit more code but it's straight forward. Then, I start writing code and I just wish I had a direct read/write method that wrapped the File <-> Blob part. I could see either way, but we should be consistent. |
jessemapel
approved these changes
Feb 23, 2021
acpaquette
approved these changes
Feb 23, 2021
jessemapel
added a commit
that referenced
this pull request
Mar 9, 2021
* Updated to new blob structure * Clean up after being able to look at all of the changes at once * Changed the history read to return a history object rather than a pointer * Addressed PR feedback * Imagepolygon Update (#4308) * Converted Imagepolygon to no longer be a subclass of blob * Updated Imagepolygon with constructor chaining * Fixed error message * Refactor OriginalLabel class to remove inheritance from Blob (#4309) * Initial refactor of OriginalLabel class * Addressed PR feedback * Fixed error message Co-authored-by: Jesse Mapel <[email protected]> * Modified Table to not inherit from Blob anymore (#4312) * Ported Table test to gtest * Table refactor * Fixed merge issue * Fixed merge issues * Removes blob inheritance from OriginalXmlLabel (#4316) * Initial refactor of OriginalLabel class * Addressed PR feedback * Fixed error message * Remove blob inheritance from originalxmllabel * Removed commented / old code * Added byte order specification * History object tests (#4314) * Adds History object tests * Updated blob tests to use a non-pointer variable * Removed gisblob from isis (#4329) * Removed blob inheritance from stretchblob (#4331) * Removed blob inheritance from stretchblob * Addressed feedback + working read/write * Updated cubestretch with the stretch blob functionality * Removed stretchBlob and updated cubestretch/cube with read write functionality * Replaced readStretchBlob with readCubeStretch * Fixed segfault * Removed debug print statements * Added check for existence of OriginalLabel before attempting to propagate * Addressed PR feedback Co-authored-by: Adam Paquette <[email protected]> * Removed StringBlob Class (#4321) * Converted StringBlob * Removed StretchBlob class * Changed Blob setData to copy * Added csminit change and docs * Removed old memcopy * History Blob Old Implementation (#4342) * Implemented old history functionality * Removed old history unit test * Fixed original label reading * Fixed Blob branch test failures (#4344) * Fixed test failures * More clean-up * Fixed marci2isis test * Blob final clean-up (#4346) * Redid Blob writing * Added docs * Review feedback * Fixed int comparison Co-authored-by: Adam Paquette <[email protected]> Co-authored-by: AustinSanders <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor OriginalLabel class to remove inheritance from Blob
Description
Refactor OriginalLabel to remove inheritance from Blob.
Related Issue
Motivation and Context
More clearly separates functionality so that we can move toward building pvl / blob / functionality independently.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Licensing
This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words: