Skip to content
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.

Sign up for GitHub

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

Expose parsers when building a pipeline through ParsingNeuralNetwork #149

Merged
merged 3 commits into from
Dec 6, 2024
Merged

Expose parsers when building a pipeline through ParsingNeuralNetwork #149

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e901c30
add getParser method to ParsingNeuralNetwork
ptoupas Dec 5, 2024
458fa13
change assertion to if with KeyError
ptoupas Dec 6, 2024
71f054d
update docstring and change the input variable name
ptoupas Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions depthai_nodes/parsing_neural_network.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ def setBackend(self, setBackend: str) -> None:
"""Sets the backend of the NeuralNetwork node."""
self._nn.setBackend(setBackend)

def getParser(self, parserID: int = 0) -> BaseParser:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts on having default value here? The other option is that you have to explicitly speciffy the parserID but I agree that in 90% of the cases users are dealing with 1parser NNs so this would be then the desired behaviour. CC: @jkbmrz @kkeroo @aljazkonec1

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also set the default value to None and if no value is provided:

  • return the parser at zero index if there is only one parser available,
  • error out if there is multiple parsers available to avoid mistakes.

BTW, is parserID a good naming? I think parser_index would be more appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having an int value set to None would work but could cause issues down the line when porting to depthai. The function name getParser implies that only one is returned and I would just update the docstring such that it explicitly states which one is returned ( i.e. something like: The first parser is returned by default if multiple parsers available).

Regarding the naming, I would switch to snake case (to be consistent with other variables/attributes). index does make slightly more sense.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave the default option, so calling getParser() returns the first one. I'm fine with both ways of doing it (Petros' way and Jakob's way).

"""Returns the parser node for the specified model head."""
assert (
ptoupas marked this conversation as resolved.
Show resolved Hide resolved
parserID in self._parsers
), f"Parser with ID {parserID} not found. Available parser IDs: {list(self._parsers.keys())}"
return self._parsers[parserID]

def setBackendProperties(self, setBackendProperties: Dict[str, str]) -> None:
"""Sets the backend properties of the NeuralNetwork node."""
self._nn.setBackendProperties(setBackendProperties)
Expand Down
Loading