-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Removed testing for delegates in different running modes from iOS Vision Tasks #4583
base: master
Are you sure you want to change the base?
Removed testing for delegates in different running modes from iOS Vision Tasks #4583
Conversation
…uired in iOS image classifier
withCode:MPPTasksErrorCodeFailedPreconditionError | ||
description:[NSString | ||
stringWithFormat:@"This method can only be called if the task is " | ||
@"running in a stream mode and an object of a class " |
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.
"an object of a class" is a bit hard to parse. Maybe just "and a delegate is provided in the task options..."
@@ -59,7 +62,20 @@ - (instancetype)initWithCalculatorGraphConfig:(CalculatorGraphConfig)graphConfig | |||
} | |||
|
|||
- (BOOL)sendPacketMap:(const PacketMap &)packetMap error:(NSError **)error { | |||
if (!_initializedWithPacketsCallback) { |
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 now means that we can initialize a task that will only ever throw errors. Is it possible to revert to the old behavior and throw an error on the initialization?
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.
Yeah, but then this whole change is unnecessary right? I mean wouldn't it suffice to just make all the delegate methods @required. Can I close this PR ?
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.
Probably. I think we can have @required
methods and tests that the delegate object is set itself (like I believe we do today).
No description provided.