Skip to content

Conversation

@ifielker
Copy link
Contributor

added support for automl-models

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Looks good in general. Just a few comments on readability.

@hiranya911 hiranya911 assigned ifielker and unassigned hiranya911 Mar 24, 2020
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of suggestions.

model_source = None
gcs_tflite_uri = data.pop('gcsTfliteUri', None)
if gcs_tflite_uri:
model_source = TFLiteGCSModelSource(gcs_tflite_uri=gcs_tflite_uri)
Copy link
Contributor

Choose a reason for hiding this comment

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

return

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

model_source = TFLiteGCSModelSource(gcs_tflite_uri=gcs_tflite_uri)
auto_ml_model = data.pop('automlModel', None)
if auto_ml_model:
model_source = TFLiteAutoMlSource(auto_ml_model=auto_ml_model)
Copy link
Contributor

Choose a reason for hiding this comment

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

return

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

def __eq__(self, other):
if isinstance(other, self.__class__):
return self._auto_ml_model == other._auto_ml_model # pylint: disable=protected-access
return self._auto_ml_model == other.auto_ml_model
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return self._auto_ml_model == other.auto_ml_model
return self.auto_ml_model == other.auto_ml_model
Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@hiranya911 hiranya911 removed their assignment Mar 26, 2020
@ifielker ifielker merged commit c4275be into mlkit-automl Mar 27, 2020
@ifielker ifielker deleted the mlkit-automl-1 branch March 27, 2020 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants