-
Notifications
You must be signed in to change notification settings - Fork 622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Metadata accessor #5650
Metadata accessor #5650
Conversation
Signed-off-by: Michał Zientkiewicz <[email protected]>
6618dd0
to
9431cdc
Compare
Signed-off-by: Michal Zientkiewicz <[email protected]>
CI MESSAGE: [18914711]: BUILD STARTED |
CI MESSAGE: [18914711]: BUILD PASSED |
|
||
template <typename Backend> | ||
auto GetProperty<Backend>::GetPropertyReader(std::string_view key) -> PropertyReader { | ||
if (key == "source_info") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if-else is fine for a couple (2-3) of properties. If we expect to have more of these I would recommend using different approach - e.g. unordered_map.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realistically, I'd expect maybe 5 qualitatively different properties. If we have anything more, it'll likely be standardized somehow and likely not even hardcoded.
Having said that - this function is called only on operator's construction - so it's not on a critical path.
Category:
New feature (non-breaking change which adds functionality)
Refactoring (Redesign of existing code that doesn't affect functionality)
Description:
This PR reimplements GetProperty to allow for:
It also introduces
DataNode.metadata
function and a shortcut forsource_info
.NOTE
layout
doesn't get a dedicated accessor, since we want to have a static (build-time) layout and dtype.Additional information:
Affected modules and functionalities:
Key points relevant for the review:
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A