NotificationsYou must be signed in to change notification settings
Fork548
Star2.8k

[CoreML] Update bindings to Xcode 11 Beta 1 - 5#6784

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

dalexsoto merged 2 commits intodotnet:xcode11fromdalexsoto:alex-coreml-b5

Aug 16, 2019

Merged

[CoreML] Update bindings to Xcode 11 Beta 1 - 5#6784

dalexsoto merged 2 commits intodotnet:xcode11fromdalexsoto:alex-coreml-b5

Aug 16, 2019

Conversation

Copy link

Member

dalexsoto commentedAug 15, 2019

No description provided.

[CoreML] Update bindings to Xcode 11 Beta 1 - 5

a562f84

dalexsoto added the note-highlightWorth calling out specifically in release notes label

Aug 15, 2019

dalexsoto added this to thexcode11 milestone

Aug 15, 2019

dalexsoto requested a review fromspouliot

August 15, 2019 19:15

spouliot approved these changes

Aug 15, 2019

View reviewed changes

Copy link

Contributor

spouliot left a comment

Choose a reason for hiding this comment

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

All minor/suggestions :)

src/coreml.cs OutdatedShow resolvedHide resolved

src/coreml.csShow resolvedHide resolved

src/coreml.cs

		interfaceMLUpdateProgressHandlers{

		[Export("initForEvents:progressHandler:completionHandler:")]
		IntPtrConstructor(MLUpdateProgressEventinterestedEvents,[NullAllowed]Action<MLUpdateContext>progressHandler,Action<MLUpdateContext>completionHandler);

Copy link

Contributor

spouliotAug 15, 2019

Choose a reason for hiding this comment

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

does[Async] make sense ?

Copy link

MemberAuthor

dalexsotoAug 15, 2019

Choose a reason for hiding this comment

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

Unfortunately this is actor

src/coreml.cs

		[Static]
		[Export("updateTaskForModelAtURL:trainingData:configuration:completionHandler:error:")]
		[return:NullAllowed]
		MLUpdateTaskCreate(NSUrlmodelUrl,IMLBatchProvidertrainingData,[NullAllowed]MLModelConfigurationconfiguration,Action<MLUpdateContext>completionHandler,[NullAllowed]outNSErrorerror);

Copy link

Contributor

spouliotAug 15, 2019

Choose a reason for hiding this comment

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

[Async] ?
I think our missing-async test will not spot such case since it's not the last parameter !?!
In fact I'm not sure if the generator would be happy...

Copy link

MemberAuthor

dalexsotoAug 15, 2019

Choose a reason for hiding this comment

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

I do not think[Async] is needed here since this creates a Task object but its completion handler is not called when this task object is finished being created but when the task is completed