Save box attributes when downloading the TFDS version of VOC #668
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
added 2 commits
February 16, 2022 16:57
`distutils` has been deprecated in Python 3.10. Fortunately, the only name we're using from there is `strtobool`, and conveniently, `attrs` has recently added a function with the same semantics and the same sets of true and false values. So just use that instead. Note that `strtobool` is still used in `setup.py`, and we can't use `attrs` there. I'm hoping that setuptools will eventually provide a replacement.
The main nuisance with this is that Datumaro represents the pose as a string, while TFDS represents it as an index, and to convert the latter into the former, we need access to the list of poses. To implement this, allow adapters access to a persistent state object that can be used to store and retrieve arbitrary data during conversion. We can then save the list of names during category creation and retrieve it when we convert box annotations.
| @frozen | ||
| class _AttributeMemberMapping: | ||
| member_name: str | ||
| attribute_name: str = field() |
There was a problem hiding this comment.
Suggested change
| attribute_name: str = field() | |
| attribute_name: str |
There was a problem hiding this comment.
I need the field object to be able to call attribute_name.default.
zhiltsov-max
previously approved these changes
Feb 16, 2022
| _AttributeMemberMapping('is_difficult', 'difficult'), | ||
| _AttributeMemberMapping('is_truncated', 'truncated'), | ||
| _AttributeMemberMapping('pose', | ||
| value_converter=lambda idx, state: state.pose_names[idx]), |
There was a problem hiding this comment.
Probably, it needs to be aligned with https://github.com/openvinotoolkit/datumaro/blob/develop/datumaro/plugins/voc_format/format.py#L50
There was a problem hiding this comment.
The only difference was the case of the string. I changed the adapter to convert the pose names to title case to better match the original dataset.
TFDS returns the pose names as lower case, but they are upper case in the original dataset. Convert them to title case so that the output of the extractor better resembles the original dataset. Remove `_SaveFeatureClassList`, since it's not useful anymore (and it's unclear whether it'll ever be useful again).
zhiltsov-max
approved these changes
Feb 17, 2022
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.
Summary
The main nuisance with this is that Datumaro represents the pose as a string, while TFDS represents it as an index, and to convert the latter into the former, we need access to the list of poses.
To implement this, allow adapters access to a persistent state object that can be used to store and retrieve arbitrary data during conversion. We can then save the list of names during category creation and retrieve it when we convert box annotations.
How to test
Checklist
developbranchLicense
Feel free to contact the maintainers if that's a concern.