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

feat: Lint with ruff #4043

Merged
merged 10 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -317,17 +317,12 @@ test-python-universal:
FEAST_USAGE=False IS_TEST=True python -m pytest -n 8 --integration sdk/python/tests

format-python:
# Sort
cd ${ROOT_DIR}/sdk/python; python -m isort feast/ tests/

# Format
cd ${ROOT_DIR}/sdk/python; python -m black --target-version py38 feast tests
cd ${ROOT_DIR}/sdk/python; python -m ruff check --fix feast/ tests/
cd ${ROOT_DIR}/sdk/python; python -m ruff format feast/ tests/

lint-python:
cd ${ROOT_DIR}/sdk/python; python -m mypy feast
cd ${ROOT_DIR}/sdk/python; python -m isort feast/ tests/ --check-only
cd ${ROOT_DIR}/sdk/python; python -m flake8 feast/ tests/
cd ${ROOT_DIR}/sdk/python; python -m black --check feast tests
# cd ${ROOT_DIR}/sdk/python; python -m mypy feast
cd ${ROOT_DIR}/sdk/python; ruff check feast/ tests/
sudohainguyen marked this conversation as resolved.
Show resolved Hide resolved

# Java

Expand Down
4 changes: 2 additions & 2 deletions docs/project/development-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ docker build -t docker-whale -f ./sdk/python/feast/infra/feature_servers/multicl
Feast Python SDK / CLI codebase:
- Conforms to [Black code style](https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html)
- Has type annotations as enforced by `mypy`
- Has imports sorted by `isort`
- Is lintable by `flake8`
- Has imports sorted by `ruff` (see [isort (I) rules](https://docs.astral.sh/ruff/rules/#isort-i))
- Is lintable by `ruff`

To ensure your Python code conforms to Feast Python code standards:
- Autoformat your code to conform to the code style:
Expand Down
43 changes: 20 additions & 23 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,24 @@ build-backend = "setuptools.build_meta"
[tool.setuptools_scm]
# Including this section is comparable to supplying use_scm_version=True in setup.py.

[tool.black]
[tool.ruff]
line-length = 88
target-version = ['py39']
include = '\.pyi?$'
exclude = '''
(
/(
\.eggs # exclude a few common directories in the
| \.git # root of the project
| \.hg
| \.mypy_cache
| \.tox
| \.venv
| _build
| buck-out
| build
| dist
| pb2.py
| \.pyi
| protos
| sdk/python/feast/embedded_go/lib
)/
)
'''
target-version = "py39"
sudohainguyen marked this conversation as resolved.
Show resolved Hide resolved
include = ["*.py", "*.pyi"]
[tool.ruff.format]
sudohainguyen marked this conversation as resolved.
Show resolved Hide resolved
# exclude a few common directories in the root of the project
exclude = [
".eggs",
".git",
".hg",
".mypy_cache",
".tox",
".venv",
"_build",
"buck-out",
"build",
"dist",
"pb2.py",
".pyi",
"protos",
"sdk/python/feast/embedded_go/lib"]
3 changes: 2 additions & 1 deletion sdk/python/feast/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
from importlib.metadata import PackageNotFoundError
from importlib.metadata import version as _version
except ModuleNotFoundError:
from importlib_metadata import PackageNotFoundError, version as _version # type: ignore
from importlib_metadata import PackageNotFoundError # type: ignore
from importlib_metadata import version as _version

from feast.infra.offline_stores.bigquery_source import BigQuerySource
from feast.infra.offline_stores.contrib.athena_offline_store.athena_source import (
Expand Down
1 change: 0 additions & 1 deletion sdk/python/feast/data_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,6 @@ def from_proto(data_source: DataSourceProto):
)

def to_proto(self) -> DataSourceProto:

schema_pb = []

if isinstance(self.schema, Dict):
Expand Down
12 changes: 6 additions & 6 deletions sdk/python/feast/diff/registry_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,12 @@ def extract_objects_for_keep_delete_update_add(
objs_to_update = {}
objs_to_add = {}

registry_object_type_to_objects: Dict[
FeastObjectType, List[Any]
] = FeastObjectType.get_objects_from_registry(registry, current_project)
registry_object_type_to_repo_contents: Dict[
FeastObjectType, List[Any]
] = FeastObjectType.get_objects_from_repo_contents(desired_repo_contents)
registry_object_type_to_objects: Dict[FeastObjectType, List[Any]] = (
FeastObjectType.get_objects_from_registry(registry, current_project)
)
registry_object_type_to_repo_contents: Dict[FeastObjectType, List[Any]] = (
FeastObjectType.get_objects_from_repo_contents(desired_repo_contents)
)

for object_type in FEAST_OBJECT_TYPES:
(
Expand Down
12 changes: 4 additions & 8 deletions sdk/python/feast/dqm/profilers/profiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@ def validate(self, dataset: pd.DataFrame) -> "ValidationReport":
...

@abc.abstractmethod
def to_proto(self):
...
def to_proto(self): ...

@classmethod
@abc.abstractmethod
def from_proto(cls, proto) -> "Profile":
...
def from_proto(cls, proto) -> "Profile": ...


class Profiler:
Expand All @@ -34,13 +32,11 @@ def analyze_dataset(self, dataset: pd.DataFrame) -> Profile:
...

@abc.abstractmethod
def to_proto(self):
...
def to_proto(self): ...

@classmethod
@abc.abstractmethod
def from_proto(cls, proto) -> "Profiler":
...
def from_proto(cls, proto) -> "Profiler": ...


class ValidationReport:
Expand Down
1 change: 0 additions & 1 deletion sdk/python/feast/embedded_go/online_features_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ def get_online_features(
request_data: Dict[str, Union[List[Any], Value_pb2.RepeatedValue]],
full_feature_names: bool = False,
):

if feature_service:
join_keys_types = self._service.GetEntityTypesMapByFeatureService(
feature_service.name
Expand Down
18 changes: 9 additions & 9 deletions sdk/python/feast/feature_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,15 @@ def get_schema(self, registry: "BaseRegistry") -> pa.Schema:
fields[join_key] = FEAST_TYPE_TO_ARROW_TYPE[entity_column.dtype]

for feature in projection.features:
fields[
f"{projection.name_to_use()}__{feature.name}"
] = FEAST_TYPE_TO_ARROW_TYPE[feature.dtype]
fields[
f"{projection.name_to_use()}__{feature.name}__timestamp"
] = PA_TIMESTAMP_TYPE
fields[
f"{projection.name_to_use()}__{feature.name}__status"
] = pa.int32()
fields[f"{projection.name_to_use()}__{feature.name}"] = (
FEAST_TYPE_TO_ARROW_TYPE[feature.dtype]
)
fields[f"{projection.name_to_use()}__{feature.name}__timestamp"] = (
PA_TIMESTAMP_TYPE
)
fields[f"{projection.name_to_use()}__{feature.name}__status"] = (
pa.int32()
)

# system columns
fields[LOG_TIMESTAMP_FIELD] = pa.timestamp("us", tz=UTC)
Expand Down
18 changes: 12 additions & 6 deletions sdk/python/feast/feature_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,8 @@ def apply(
views_to_update = [
ob
for ob in objects
if (
if
(
# BFVs are not handled separately from FVs right now.
(isinstance(ob, FeatureView) or isinstance(ob, BatchFeatureView))
and not isinstance(ob, StreamFeatureView)
Expand Down Expand Up @@ -950,7 +951,9 @@ def apply(
validation_references.name, project=self.project, commit=False
)

tables_to_delete: List[FeatureView] = views_to_delete + sfvs_to_delete if not partial else [] # type: ignore
tables_to_delete: List[FeatureView] = (
views_to_delete + sfvs_to_delete if not partial else []
) # type: ignore
sudohainguyen marked this conversation as resolved.
Show resolved Hide resolved
tables_to_keep: List[FeatureView] = views_to_update + sfvs_to_update # type: ignore

self._get_provider().update_infra(
Expand Down Expand Up @@ -1575,7 +1578,10 @@ def _get_online_features(

num_rows = _validate_entity_values(entity_proto_values)
_validate_feature_refs(_feature_refs, full_feature_names)
(grouped_refs, grouped_odfv_refs,) = _group_feature_refs(
(
grouped_refs,
grouped_odfv_refs,
) = _group_feature_refs(
_feature_refs,
requested_feature_views,
requested_on_demand_feature_views,
Expand Down Expand Up @@ -1728,9 +1734,9 @@ def _get_entity_maps(
)
entity_name_to_join_key_map[entity_name] = join_key
for entity_column in feature_view.entity_columns:
entity_type_map[
entity_column.name
] = entity_column.dtype.to_value_type()
entity_type_map[entity_column.name] = (
entity_column.dtype.to_value_type()
)

return (
entity_name_to_join_key_map,
Expand Down
1 change: 0 additions & 1 deletion sdk/python/feast/infra/materialization/aws_lambda/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ def handler(event, context):
print("Received event: " + json.dumps(event, indent=2), flush=True)

try:

config_base64 = event[FEATURE_STORE_YAML_ENV_NAME]

config_bytes = base64.b64decode(config_base64)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,19 @@ class MaterializationJob(ABC):
task: MaterializationTask

@abstractmethod
def status(self) -> MaterializationJobStatus:
...
def status(self) -> MaterializationJobStatus: ...

@abstractmethod
def error(self) -> Optional[BaseException]:
...
def error(self) -> Optional[BaseException]: ...

@abstractmethod
def should_be_retried(self) -> bool:
...
def should_be_retried(self) -> bool: ...

@abstractmethod
def job_id(self) -> str:
...
def job_id(self) -> str: ...

@abstractmethod
def url(self) -> Optional[str]:
...
def url(self) -> Optional[str]: ...


class BatchMaterializationEngine(ABC):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

import pyarrow as pa
import pyarrow.parquet as pq

from bytewax.dataflow import Dataflow # type: ignore
from bytewax.execution import cluster_main
from bytewax.inputs import ManualInputConfig
from bytewax.outputs import ManualOutputConfig

from feast import FeatureStore, FeatureView, RepoConfig
from feast.utils import _convert_arrow_to_proto, _run_pyarrow_field_mapping

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
from typing import Callable, List, Literal, Sequence, Union

import yaml
from kubernetes import client
from kubernetes import client, utils
from kubernetes import config as k8s_config
from kubernetes import utils
from kubernetes.client.exceptions import ApiException
from kubernetes.utils import FailToCreateError
from pydantic import StrictStr
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from typing import Callable, List, Literal, Optional, Sequence, Union, cast

import dill
import pandas
import pandas as pd
import pyarrow
from tqdm import tqdm
Expand Down Expand Up @@ -201,7 +200,6 @@ class _SparkSerializedArtifacts:

@classmethod
def serialize(cls, feature_view, repo_config):

# serialize to proto
feature_view_proto = feature_view.to_proto().SerializeToString()

Expand Down
8 changes: 3 additions & 5 deletions sdk/python/feast/infra/materialization/snowflake_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ def teardown_infra(
fvs: Sequence[Union[BatchFeatureView, StreamFeatureView, FeatureView]],
entities: Sequence[Entity],
):

stage_path = f'"{self.repo_config.batch_engine.database}"."{self.repo_config.batch_engine.schema_}"."feast_{project}"'
with GetSnowflakeConnection(self.repo_config.batch_engine) as conn:
query = f"DROP STAGE IF EXISTS {stage_path}"
Expand Down Expand Up @@ -230,8 +229,9 @@ def _materialize_one(
project: str,
tqdm_builder: Callable[[int], tqdm],
):
assert isinstance(feature_view, BatchFeatureView) or isinstance(
feature_view, FeatureView
assert (
isinstance(feature_view, BatchFeatureView)
or isinstance(feature_view, FeatureView)
), "Snowflake can only materialize FeatureView & BatchFeatureView feature view types."

entities = []
Expand Down Expand Up @@ -350,7 +350,6 @@ def generate_snowflake_materialization_query(
feature_batch: list,
project: str,
) -> str:

if feature_view.batch_source.created_timestamp_column:
fv_created_str = f',"{feature_view.batch_source.created_timestamp_column}"'
else:
Expand Down Expand Up @@ -477,7 +476,6 @@ def materialize_to_external_online_store(
feature_view: Union[StreamFeatureView, FeatureView],
pbar: tqdm,
) -> None:

feature_names = [feature.name for feature in feature_view.features]

with GetSnowflakeConnection(repo_config.batch_engine) as conn:
Expand Down
6 changes: 3 additions & 3 deletions sdk/python/feast/infra/offline_stores/bigquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ class BigQueryOfflineStoreConfig(FeastConfigBaseModel):
gcs_staging_location: Optional[str] = None
""" (optional) GCS location used for offloading BigQuery results as parquet files."""

table_create_disposition: Literal[
"CREATE_NEVER", "CREATE_IF_NEEDED"
] = "CREATE_IF_NEEDED"
table_create_disposition: Literal["CREATE_NEVER", "CREATE_IF_NEEDED"] = (
"CREATE_IF_NEEDED"
)
""" (optional) Specifies whether the job is allowed to create new tables. The default value is CREATE_IF_NEEDED.
Custom constraint for table_create_disposition. To understand more, see:
https://cloud.google.com/bigquery/docs/reference/rest/v2/Job#JobConfigurationLoad.FIELDS.create_disposition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ def get_historical_features(

@contextlib.contextmanager
def query_generator() -> Iterator[str]:

table_name = offline_utils.get_temp_entity_table_name()

_upload_entity_df(entity_df, athena_client, config, s3_resource, table_name)
Expand Down Expand Up @@ -240,7 +239,6 @@ def query_generator() -> Iterator[str]:
try:
yield query
finally:

# Always clean up the temp Athena table
aws_utils.execute_athena_query(
athena_client,
Expand Down Expand Up @@ -423,7 +421,6 @@ def persist(

@log_exceptions_and_usage
def to_athena(self, table_name: str) -> None:

if self.on_demand_feature_views:
transformed_df = self.to_df()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ def __init__(

@staticmethod
def from_proto(storage_proto: SavedDatasetStorageProto) -> SavedDatasetStorage:

return SavedDatasetAthenaStorage(
table_ref=AthenaOptions.from_proto(storage_proto.athena_storage).table
)
Expand Down
Loading
Loading