Skip to content

Commit 9ea1f52

Browse files
yeejian-tanwoopimjuanleonard
authored
Enable Mypy static type checking (feast-dev#749)
* Enable Mypy support * Remove proto compilation from linting * Remove broken rebase imports * Finalize Mypy tests and fix broken imports * Remove CLI duplication of methods * Fix failing undefined responses * Remove comment about requiring protos to be compiled * Enable proto compilation for Python linting in GH action Co-authored-by: Tan Yee Jian <tanyeejian@gmail.com> Co-authored-by: Willem Pienaar <git@willem.co> Co-authored-by: Julio Anthony Leonard <julioanthonyleonard@gmail.com>
1 parent 55f0dff commit 9ea1f52

19 files changed

Lines changed: 154 additions & 206 deletions

‎.github/workflows/code_standards.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ jobs:
1818
- uses: actions/checkout@v2
1919
- name: install dependencies
2020
run: make install-python-ci-dependencies
21+
- name: compile protos
22+
run: make compile-protos-python
2123
- name: lint python
2224
run: make lint-python
2325

‎Makefile‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
#
1+
#
22
# Copyright 2019 The Feast Authors
3-
#
3+
#
44
# Licensed under the Apache License, Version 2.0 (the "License");
55
# you may not use this file except in compliance with the License.
66
# You may obtain a copy of the License at
7-
#
7+
#
88
# https://www.apache.org/licenses/LICENSE-2.0
9-
#
9+
#
1010
# Unless required by applicable law or agreed to in writing, software
1111
# distributed under the License is distributed on an "AS IS" BASIS,
1212
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -73,8 +73,7 @@ format-python:
7373
cd ${ROOT_DIR}/sdk/python; black --target-version py37 feast tests
7474

7575
lint-python:
76-
# TODO: This mypy test needs to be re-enabled and all failures fixed
77-
#cd ${ROOT_DIR}/sdk/python; mypy feast/ tests/
76+
cd ${ROOT_DIR}/sdk/python; mypy feast/ tests/
7877
cd ${ROOT_DIR}/sdk/python; flake8 feast/ tests/
7978
cd ${ROOT_DIR}/sdk/python; black --check feast tests
8079

@@ -104,7 +103,7 @@ build-push-docker:
104103
@$(MAKE) push-core-docker registry=$(REGISTRY) version=$(VERSION)
105104
@$(MAKE) push-serving-docker registry=$(REGISTRY) version=$(VERSION)
106105
@$(MAKE) push-ci-docker registry=$(REGISTRY)
107-
106+
108107
build-docker: build-core-docker build-serving-docker build-ci-docker
109108

110109
push-core-docker:

‎sdk/python/feast/cli.py‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import json
1616
import logging
1717
import sys
18+
from typing import Dict, List
1819

1920
import click
2021
import pkg_resources
@@ -120,28 +121,12 @@ def feature():
120121
pass
121122

122123

123-
def _get_features_labels_dict(label_str: str):
124-
"""
125-
Converts CLI input labels string to dictionary format if provided string is valid.
126-
"""
127-
labels_dict = {}
128-
labels_kv = label_str.split(",")
129-
if label_str == "":
130-
return labels_dict
131-
if len(labels_kv) % 2 == 1:
132-
raise ValueError("Uneven key-value label pairs were entered")
133-
for k, v in zip(labels_kv[0::2], labels_kv[1::2]):
134-
labels_dict[k] = v
135-
return labels_dict
136-
137-
138-
def _get_features_entities(entities_str: str):
124+
def _convert_entity_string_to_list(entities_str: str) -> List[str]:
139125
"""
140126
Converts CLI input entities string to list format if provided string is valid.
141127
"""
142-
entities_list = []
143128
if entities_str == "":
144-
return entities_list
129+
return []
145130
return entities_str.split(",")
146131

147132

@@ -173,8 +158,8 @@ def feature_list(project: str, entities: str, labels: str):
173158
"""
174159
feast_client = Client() # type: Client
175160

176-
entities_list = _get_features_entities(entities)
177-
labels_dict = _get_features_labels_dict(labels)
161+
entities_list = _convert_entity_string_to_list(entities)
162+
labels_dict: Dict[str, str] = _get_labels_dict(labels)
178163

179164
table = []
180165
for feature_ref, feature in feast_client.list_features_by_ref(
@@ -195,11 +180,11 @@ def feature_set():
195180
pass
196181

197182

198-
def _get_labels_dict(label_str: str):
183+
def _get_labels_dict(label_str: str) -> Dict[str, str]:
199184
"""
200185
Converts CLI input labels string to dictionary format if provided string is valid.
201186
"""
202-
labels_dict = {}
187+
labels_dict: Dict[str, str] = {}
203188
labels_kv = label_str.split(",")
204189
if label_str == "":
205190
return labels_dict

‎sdk/python/feast/client.py‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@
9090

9191
_logger = logging.getLogger(__name__)
9292

93-
CPU_COUNT = os.cpu_count() # type: int
93+
CPU_COUNT: int = len(os.sched_getaffinity(0))
9494

9595

9696
class Client:
@@ -123,9 +123,9 @@ def __init__(self, options: Optional[Dict[str, str]] = None, **kwargs):
123123
options = dict()
124124
self._config = Config(options={**options, **kwargs})
125125

126-
self._core_service_stub: CoreServiceStub = None
127-
self._serving_service_stub: ServingServiceStub = None
128-
self._auth_metadata = None
126+
self._core_service_stub: Optional[CoreServiceStub] = None
127+
self._serving_service_stub: Optional[ServingServiceStub] = None
128+
self._auth_metadata: Optional[grpc.AuthMetadataPlugin] = None
129129

130130
# Configure Auth Metadata Plugin if auth is enabled
131131
if self._config.getboolean(CONFIG_CORE_ENABLE_AUTH_KEY):
@@ -475,7 +475,7 @@ def get_feature_set(
475475
raise ValueError("No project has been configured.")
476476

477477
try:
478-
get_feature_set_response = self._core_service_stub.GetFeatureSet(
478+
get_feature_set_response = self._core_service.GetFeatureSet(
479479
GetFeatureSetRequest(project=project, name=name.strip()),
480480
metadata=self._get_grpc_metadata(),
481481
) # type: GetFeatureSetResponse
@@ -719,9 +719,9 @@ def list_ingest_jobs(
719719
)
720720
request = ListIngestionJobsRequest(filter=list_filter)
721721
# make list request & unpack response
722-
response = self._core_service_stub.ListIngestionJobs(request)
722+
response = self._core_service_stub.ListIngestionJobs(request) # type: ignore
723723
ingest_jobs = [
724-
IngestJob(proto, self._core_service_stub) for proto in response.jobs
724+
IngestJob(proto, self._core_service_stub) for proto in response.jobs # type: ignore
725725
]
726726
return ingest_jobs
727727

@@ -737,7 +737,7 @@ def restart_ingest_job(self, job: IngestJob):
737737
"""
738738
request = RestartIngestionJobRequest(id=job.id)
739739
try:
740-
self._core_service_stub.RestartIngestionJob(request)
740+
self._core_service.RestartIngestionJob(request) # type: ignore
741741
except grpc.RpcError as e:
742742
raise grpc.RpcError(e.details())
743743

@@ -753,7 +753,7 @@ def stop_ingest_job(self, job: IngestJob):
753753
"""
754754
request = StopIngestionJobRequest(id=job.id)
755755
try:
756-
self._core_service_stub.StopIngestionJob(request)
756+
self._core_service.StopIngestionJob(request) # type: ignore
757757
except grpc.RpcError as e:
758758
raise grpc.RpcError(e.details())
759759

@@ -817,11 +817,12 @@ def ingest(
817817
while True:
818818
if timeout is not None and time.time() - current_time >= timeout:
819819
raise TimeoutError("Timed out waiting for feature set to be ready")
820-
feature_set = self.get_feature_set(name)
820+
fetched_feature_set: Optional[FeatureSet] = self.get_feature_set(name)
821821
if (
822-
feature_set is not None
823-
and feature_set.status == FeatureSetStatus.STATUS_READY
822+
fetched_feature_set is not None
823+
and fetched_feature_set.status == FeatureSetStatus.STATUS_READY
824824
):
825+
feature_set = fetched_feature_set
825826
break
826827
time.sleep(3)
827828

@@ -944,7 +945,7 @@ def get_statistics(
944945
if end_date is not None:
945946
request.end_date.CopyFrom(Timestamp(seconds=int(end_date.timestamp())))
946947

947-
return self._core_service_stub.GetFeatureStatistics(
948+
return self._core_service.GetFeatureStatistics(
948949
request
949950
).dataset_feature_statistics_list
950951

‎sdk/python/feast/feature_set.py‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from google.protobuf.duration_pb2 import Duration
2323
from google.protobuf.json_format import MessageToDict, MessageToJson
2424
from google.protobuf.message import Message
25+
from google.protobuf.timestamp_pb2 import Timestamp
2526
from pandas.api.types import is_datetime64_ns_dtype
2627
from pyarrow.lib import TimestampType
2728

@@ -62,20 +63,20 @@ def __init__(
6263
self._project = project
6364
self._fields = OrderedDict() # type: Dict[str, Field]
6465
if features is not None:
65-
self.features = features
66+
self.features: Optional[List[Feature]] = features
6667
if entities is not None:
6768
self.entities = entities
6869
if source is None:
6970
self._source = None
7071
else:
7172
self._source = source
7273
if labels is None:
73-
self._labels = OrderedDict()
74+
self._labels = OrderedDict() # type: MutableMapping[str, str]
7475
else:
7576
self._labels = labels
7677
self._max_age = max_age
7778
self._status = None
78-
self._created_timestamp = None
79+
self._created_timestamp: Optional[Timestamp] = None
7980

8081
def __eq__(self, other):
8182
if not isinstance(other, FeatureSet):
@@ -314,7 +315,7 @@ def drop(self, name: str):
314315
"""
315316
del self._fields[name]
316317

317-
def _add_fields(self, fields: List[Field]):
318+
def _add_fields(self, fields):
318319
"""
319320
Adds multiple Fields to a Feature Set
320321
@@ -379,8 +380,9 @@ def infer_fields_from_df(
379380

380381
# Create dictionary of fields that will not be inferred (manually set)
381382
provided_fields = OrderedDict()
383+
fields = _create_field_list(entities, features)
382384

383-
for field in entities + features:
385+
for field in fields:
384386
if not isinstance(field, Field):
385387
raise Exception(f"Invalid field object type provided {type(field)}")
386388
if field.name not in provided_fields:
@@ -518,8 +520,9 @@ def infer_fields_from_pa(
518520

519521
# Create dictionary of fields that will not be inferred (manually set)
520522
provided_fields = OrderedDict()
523+
fields = _create_field_list(entities, features)
521524

522-
for field in entities + features:
525+
for field in fields:
523526
if not isinstance(field, Field):
524527
raise Exception(f"Invalid field object type provided {type(field)}")
525528
if field.name not in provided_fields:
@@ -835,7 +838,7 @@ def from_proto(cls, feature_set_proto: FeatureSetProto):
835838
if len(feature_set_proto.spec.project) == 0
836839
else feature_set_proto.spec.project,
837840
)
838-
feature_set._status = feature_set_proto.meta.status
841+
feature_set._status = feature_set_proto.meta.status # type: ignore
839842
feature_set._created_timestamp = feature_set_proto.meta.created_timestamp
840843
return feature_set
841844

@@ -1041,3 +1044,29 @@ def _infer_pd_column_type(column, series, rows_to_sample):
10411044
dtype = current_dtype
10421045

10431046
return dtype
1047+
1048+
1049+
def _create_field_list(entities: List[Entity], features: List[Feature]) -> List[Field]:
1050+
"""
1051+
Convert entities and features List to Field List
1052+
1053+
Args:
1054+
entities: List of Entity Objects
1055+
features: List of Features Objects
1056+
1057+
1058+
Returns:
1059+
List[Field]:
1060+
List of field from entities and features combined
1061+
"""
1062+
fields: List[Field] = []
1063+
1064+
for entity in entities:
1065+
if isinstance(entity, Field):
1066+
fields.append(entity)
1067+
1068+
for feature in features:
1069+
if isinstance(feature, Field):
1070+
fields.append(feature)
1071+
1072+
return fields

0 commit comments

Comments
 (0)