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.

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

[v6] Implement ML-Kit Natural Language #2117

Merged
merged 37 commits into from Jun 2, 2019
Merged

[v6] Implement ML-Kit Natural Language #2117

merged 37 commits into from Jun 2, 2019

Conversation

@Salakar
Copy link
Member

Salakar commented May 7, 2019

[ml-kit-natural-language]

firebase.ml().nlp

  • JS
  • iOS
  • Android
    • Language ID
    • Translate Language
    • Smart Replies
  • Tests (95-100% coverage)
  • 100% Firebase API coverage
  • Flow Types
  • Typescript Definitions
  • Update package README

firebase.json supported flags

Added support for the following flags to support optional models (so they're not all bundled together into your app unless you decide you need them):

{
  "react-native": {
    "ml_natural_language_id_model" : true,
    "ml_natural_language_translate_model" : true,
    "ml_natural_language_smart_reply_model" : true
  }
}
[skip ci]
@RWOverdijk

This comment has been minimized.

Copy link

RWOverdijk commented May 8, 2019

yesyesyesyesyesyesyesyesyesyesyesyesyesyesYESYEEEEES

Cool.

Salakar and others added 13 commits May 8, 2019
[skip ci]
[android] update to latest firebase sdks - previous release broken (#2122)

[skip ci]
…eact-native-firebase into @salakar/v6/mlkit

# Conflicts:
#	packages/analytics/android/src/main/AndroidManifest.xml
#	packages/analytics/android/src/main/java/io/invertase/firebase/analytics/ReactNativeFirebaseAnalyticsModule.java
#	packages/functions/android/src/main/java/io/invertase/firebase/functions/ReactNativeFirebaseFunctionsModule.java

[skip ci]
[skip ci]
[skip ci]
@Salakar

This comment has been minimized.

Copy link
Member Author

Salakar commented May 12, 2019

Hey @RWOverdijk, @sejas, @zhigang1992, @gianpaj, @fungilation and others interested in ML Kit.

If you can, please could you provide your feedback/thoughts on the API for the ML Kit Natural Language integration, as I'm currently unsure if this API is clear enough or if anything needs changing?

// -- LANGUAGE_LANGUAGE_ID
// --------------------------
firebase.ml().language().identifyLanguage(text): Promise<string>;
// -- LANGUAGE_SMART_REPLIES
// --------------------------
firebase.ml().language().newSmartReplyConversation(): SmartReplyConversation;

//    -> SmartReplyConversation
//      -> destroy(): void;
//      -> clear(): void;
//      -> getSuggestedReplies(): Promise<SuggestedReply[]>;
//      -> addLocalUserMessage(message, timestamp = Date.now()): void;
//      -> addRemoteUserMessage(message, timestamp = Date.now(), remoteUserId): void;
// -- LANGUAGE_TRANSLATE
// --------------------------
firebase.ml().language().translateText(text, options): Promise<string>;

firebase.ml().language().translateGetAvailableModels(): Promise<Object[]>;

firebase.ml().language().translateDeleteDownloadedModel(languageId: Number): Promise<void>;

firebase.ml().language().translateDownloadRemoteModel(languageId: Number, downloadConditions: Object): Promise<void>;
@invertase invertase deleted a comment from codecov bot May 12, 2019
@RWOverdijk

This comment has been minimized.

Copy link

RWOverdijk commented May 12, 2019

@Salakar This reply is me being in the middle of something and the quality might be poor so... Sorry if that's the case 😅

My first question would be, how close is it to the firebase APIs themselves? I'm not working with react-native-camera and its MLKit implementation for detecting faces (ick) and there's almost no resemblance making it hard to debug stuff.

Secondly I wonder why every subset is a function. It feels like a lot is being constructed that I wouldn't use (but I might be off).

Third, I don't think I dig the translate prefix. Are you translating a delete of a downloaded modal, or are you deleting a downloaded model in the namespace translate? To follow the pattern you'd have to use .translate() and then .deleteDownloadedModel for example.

  • .translate().text()
  • .translate().deleteDownloadedModel()
  • .translate().availableModels()

etc.

I hope this helps.

@invertase invertase deleted a comment from codecov bot May 12, 2019
@gianpaj

This comment has been minimized.

Copy link
Contributor

gianpaj commented May 13, 2019

I don't know anything about these natural language methods in ML kit.
But when the Vision API will be developed I would probably look this official Flutter Firebase repo:
https://github.com/flutter/plugins/tree/master/packages/firebase_ml_vision
and this example app
https://github.com/firebase/mlkit-custom-image-classifier

Would be great to make an example app like that.
I'll be happy to help out with that one we have a draft API working

@Salakar

This comment has been minimized.

Copy link
Member Author

Salakar commented May 16, 2019

Preview vid of smart replies working, if you want to sneak peak: https://twitter.com/mikediarmid/status/1128837402481635331


Will come back to the API discussion, getting a basic JS API in place and completing Android / iOS code first - can re-work JS API if needed once native stable.

Salakar and others added 4 commits May 16, 2019
-
[skip ci]
-
[skip ci]
[skip ci]
[skip ci]
@Salakar Salakar changed the title [v6] Implement ML-Kit [v6] Implement ML-Kit Natural Language May 27, 2019
Salakar added 2 commits May 27, 2019
[skip ci]
@invertase invertase deleted a comment from codecov bot Jun 2, 2019
@invertase invertase deleted a comment from codecov bot Jun 2, 2019
@invertase invertase deleted a comment from codecov bot Jun 2, 2019
@invertase invertase deleted a comment from codecov bot Jun 2, 2019
@invertase invertase deleted a comment from codecov bot Jun 2, 2019
@invertase invertase deleted a comment from codecov bot Jun 2, 2019
@invertase invertase deleted a comment from codecov bot Jun 2, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jun 2, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@9083803). Click here to learn what that means.
The diff coverage is 97.09%.

@@            Coverage Diff            @@
##             master    #2117   +/-   ##
=========================================
  Coverage          ?   90.37%           
=========================================
  Files             ?       42           
  Lines             ?      934           
  Branches          ?        0           
=========================================
  Hits              ?      844           
  Misses            ?       90           
  Partials          ?        0
1 similar comment
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jun 2, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@9083803). Click here to learn what that means.
The diff coverage is 97.09%.

@@            Coverage Diff            @@
##             master    #2117   +/-   ##
=========================================
  Coverage          ?   90.37%           
=========================================
  Files             ?       42           
  Lines             ?      934           
  Branches          ?        0           
=========================================
  Hits              ?      844           
  Misses            ?       90           
  Partials          ?        0
@Salakar Salakar merged commit 21a1686 into master Jun 2, 2019
11 of 12 checks passed
11 of 12 checks passed
Labeler Labeler
Details
Synchronize or Opened Synchronize or Opened
Details
WIP Ready for review
Details
ci/circleci: analyse Your tests passed on CircleCI!
Details
ci/circleci: android-build-debug Your tests passed on CircleCI!
Details
ci/circleci: android-build-release Your tests passed on CircleCI!
Details
ci/circleci: android-test-e2e Your tests passed on CircleCI!
Details
ci/circleci: checkout-code Your tests passed on CircleCI!
Details
ci/circleci: ios-test-e2e Your tests passed on CircleCI!
Details
codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
Details
license/cla Contributor License Agreement is signed.
Details
@Salakar Salakar deleted the @salakar/v6/mlkit branch Aug 5, 2019
7patricia pushed a commit to uphold-forks/react-native-firebase that referenced this pull request Nov 29, 2019
 - Implement ML-Kit Natural Language (invertase#2117)
 - Includes additional refactor changes across other modules for internals api reworking
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.