Implementation of Model, ModelFormat, TFLiteModelSource and subclasses

[ifielker]

Implementation of Model, ModelFormat, TFLiteFormat, TFLiteModelSource, and TFLiteGCSModelSource

[hiranya911]

Pointed out a few things that can be improved.

[hiranya911]

You can save a few lines if you get these validators to return the validated value.

self._data['displayName'] = _validate_display_name(display_name)

[ifielker]

done

[hiranya911]

Looks pretty solid. Mostly just nits. However, consider this pattern when composing types:

class ModelFormat:
    def __init__(model_source, **kwargs):
       self._data = kwargs
       self.model_source = model_source

    def as_dict(self):
       # validate _data and model_source if necessary
       copy = dict(self._data)
       copy.update(self.model_source.as_dict())
       return copy

class TFLiteGCSModelSource:
        def __init__(self, gcs_tflite_uri):
             self.gcs_tflite_uri = gcs_tflite_uri

        def as_dict(self):
             # validate uri
             return {'gcsTfliteUri': self.gcs_tflite_uri}

Pros:

• Knowledge of individual fields get encapsulated in the corresponding subclasses. So it's a better use of polymorphism.
• No explicit setters needed.

Cons:

• Validation is deferred until as_dict() is called. So calling a constructor with invalid params won't raise any errors. But trying to call an RPC will trigger the error (we use this pattern in FCM and it works well in practice).

[hiranya911]

Looks pretty good. Just a couple of ideas on how to simplify the big constructors.

[hiranya911]

Remove "String -" part.

[ifielker]

done

[hiranya911]

This constructor seems to be doing a lot. Can we simplify as follows?

def __init__(self, display_name=None, tags=None, model_format=None):
    self._display_name = None
    self._model_format = None
    self._tags = None
    self._data = {}

    if display_name is not None:
        self.display_name = display_name
    if tags is not None:
        self.tags = tags
    if model_format is not None:
        self.model_format = model_format

@classmethod
def from_dict(cls, data):
    model_format = None
    tflite_format = data.pop('tfliteModel', None)
    if tflite_format:
        model_format = TFLiteFormat(**tflite_format)
    model = Model(data.pop('displayName', None), model_format)
    model._data = data
    return model

[ifielker]

done

[hiranya911]

self._data.get('createTime', {}).get('seconds')

[ifielker]

done

[hiranya911]

Same here

[ifielker]

done

[hiranya911]

Simplify here too by adding a from_dict() method that handles json dict extraction

[ifielker]

done

[hiranya911]

Thanks. LGTM 👍