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.

Sign up for GitHub

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

Jump to bottom

chore: Refactoring to fix mypy linting #3947

Closed
wants to merge 13 commits into from
Closed

chore: Refactoring to fix mypy linting #3947

wants to merge 13 commits into from

Conversation

bushwhackr
Copy link
Contributor

What this PR does / why we need it: mypy lint fixes, refer to: #3938

subset of mypy linting fixes when mypy version was upgraded in #3938.

The PR is getting alittle too long, so I'm breaking into another PR for the final fixes.

~/repo/feast ❯ cd sdk/python/; mypy                                                                                                                                                                                                                                                                 
feast/feature_store.py:1009: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
feast/repo_operations.py:249: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
feast/repo_operations.py:263: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
feast/infra/utils/snowflake/snowflake_utils.py:124: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/utils/feature_records.py:335: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/unit/test_registry_server.py:22: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/unit/test_registry_server.py:33: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/unit/test_registry_server.py:51: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
feast/infra/materialization/contrib/bytewax/bytewax_materialization_dataflow.py:56: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/integration/registration/test_universal_types.py:337: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/integration/registration/test_universal_types.py:356: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/integration/feature_repos/repo_configuration.py:169: error: Value expression in dictionary comprehension has incompatible type "Tuple[Union[str, Dict[Any, Any], None], Optional[Type[OnlineStoreCreator]]]"; expected type "Tuple[Union[str, Dict[str, str]], Optional[Type[OnlineStoreCreator]]]"  [misc]
tests/integration/feature_repos/repo_configuration.py:350: error: Argument "driver_odfv" to "UniversalFeatureViews" has incompatible type "Optional[OnDemandFeatureView]"; expected "OnDemandFeatureView"  [arg-type]
tests/integration/feature_repos/repo_configuration.py:351: error: Argument 1 to "conv_rate_plus_100_feature_view" has incompatible type "List[Any]"; expected "Dict[str, Union[RequestSource, FeatureView]]"  [arg-type]
tests/integration/feature_repos/repo_configuration.py:375: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/integration/feature_repos/repo_configuration.py:412: error: Incompatible types in assignment (expression has type "FeastConfigBaseModel", variable has type "Union[str, Dict[Any, Any], None]")  [assignment]
tests/integration/feature_repos/repo_configuration.py:415: error: Incompatible types in assignment (expression has type "Union[str, Dict[Any, Any], None]", variable has type "FeastConfigBaseModel")  [assignment]
tests/integration/feature_repos/repo_configuration.py:422: error: Missing named argument "feature_logging" for "AwsLambdaFeatureServerConfig"  [call-arg]
tests/integration/feature_repos/repo_configuration.py:431: error: Incompatible types in assignment (expression has type "LocalFeatureServerConfig", variable has type "AwsLambdaFeatureServerConfig")  [assignment]
tests/integration/feature_repos/repo_configuration.py:446: error: Missing named argument "registry_store_type" for "RegistryConfig"  [call-arg]
tests/integration/feature_repos/repo_configuration.py:446: error: Missing named argument "s3_additional_kwargs" for "RegistryConfig"  [call-arg]
tests/conftest.py:222: error: Incompatible types in assignment (expression has type "List[Tuple[str, Any]]", variable has type "Optional[List[Optional[Tuple[str, Type[DataSourceCreator]]]]]")  [assignment]
tests/conftest.py:237: error: Incompatible types in assignment (expression has type "dict_values[str, Tuple[Union[str, Dict[str, str]], Optional[Type[OnlineStoreCreator]]]]", variable has type "Optional[List[Optional[Tuple[Union[str, Dict[str, str]], Optional[Type[OnlineStoreCreator]]]]]]")  [assignment]
tests/conftest.py:262: error: "None" object is not iterable  [misc]
tests/conftest.py:263: error: "None" object is not iterable  [misc]
tests/conftest.py:348: error: Too many arguments for "create_data_source" of "DataSourceCreator"  [call-arg]
tests/unit/online_store/test_online_retrieval.py:139: error: Missing named argument "registry_store_type" for "RegistryConfig"  [call-arg]
tests/unit/online_store/test_online_retrieval.py:139: error: Missing named argument "s3_additional_kwargs" for "RegistryConfig"  [call-arg]
tests/unit/online_store/test_online_retrieval.py:202: error: Missing named argument "registry_store_type" for "RegistryConfig"  [call-arg]
tests/unit/online_store/test_online_retrieval.py:202: error: Missing named argument "s3_additional_kwargs" for "RegistryConfig"  [call-arg]
tests/unit/infra/test_local_registry.py:41: error: Missing named argument "registry_store_type" for "RegistryConfig"  [call-arg]
tests/unit/infra/test_local_registry.py:41: error: Missing named argument "s3_additional_kwargs" for "RegistryConfig"  [call-arg]
tests/unit/cli/test_cli_chdir.py:21: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
tests/unit/cli/test_cli_chdir.py:24: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
tests/unit/cli/test_cli_chdir.py:28: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
tests/unit/cli/test_cli_chdir.py:37: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
tests/unit/cli/test_cli_chdir.py:47: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
tests/unit/cli/test_cli_chdir.py:52: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
tests/unit/cli/test_cli_chdir.py:55: error: List item 1 has incompatible type "Path"; expected "str"  [list-item]
tests/integration/registration/test_registry.py:45: error: Missing named argument "registry_store_type" for "RegistryConfig"  [call-arg]
tests/integration/registration/test_registry.py:45: error: Missing named argument "s3_additional_kwargs" for "RegistryConfig"  [call-arg]
tests/integration/registration/test_registry.py:56: error: Missing named argument "registry_store_type" for "RegistryConfig"  [call-arg]
tests/integration/registration/test_registry.py:56: error: Missing named argument "s3_additional_kwargs" for "RegistryConfig"  [call-arg]
tests/integration/offline_store/test_universal_historical_retrieval.py:170: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/integration/offline_store/test_s3_custom_endpoint.py:43: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/integration/offline_store/test_offline_write.py:134: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
feast/infra/offline_stores/contrib/trino_offline_store/tests/data_source.py:128: error: Missing named argument "user" for "TrinoOfflineStoreConfig"  [call-arg]
feast/infra/offline_stores/contrib/trino_offline_store/tests/data_source.py:128: error: Missing named argument "auth" for "TrinoOfflineStoreConfig"  [call-arg]
feast/infra/offline_stores/contrib/postgres_offline_store/tests/data_source.py:112: error: Return type "Dict[str, str]" of "create_online_store" incompatible with return type "FeastConfigBaseModel" in supertype "OnlineStoreCreator"  [override]
tests/integration/e2e/test_python_feature_server.py:131: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
Found 34 errors in 8 files (checked 430 source files)

Which issue(s) this PR fixes:

Fixes #

@bushwhackr
allowing follow-imports + fixing on_demand_feature_view.py
68d18d2 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
creating base type SnowflakeStoreConfig to fix circular imports
73bba2c 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
fixing circular imports
d80acd8 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
converting create_data_source to take in arbitrary arguments
3ecff66 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
lint fix on base function create_online_store
39fb45b 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
lint fix on ingest_stream_feature_view
83818e0 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
lint fixes in configuration
2fa14b7 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
isort linting
a96c266 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
mypy checking of all files in sdk/python folder
f0f7fec 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
checking only feast directory for mypy errors
28010b7 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
allow extras for FeastConfigBaseModel
5dbe13d 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
fixing tests
004949d 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
@bushwhackr
adding extra field for online store config
2696cc3 
Signed-off-by: Chester Ong <chester.ong.ch@gmail.com>
bushwhackr
bushwhackr commented Feb 13, 2024
View reviewed changes
sdk/python/tests/integration/feature_repos/universal/data_source_creator.py
Comment on lines 16 to 20
def create_data_source(
self,
df: pd.DataFrame,
destination_name: str,
event_timestamp_column="ts",
created_timestamp_column="created_ts",
field_mapping: Optional[Dict[str, str]] = None,
timestamp_field: Optional[str] = None,
**kwargs,
) -> DataSource:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

functionality change: due to create_data_source needing an indeterminate number of arguments. The base class should accept kwargs and the concrete class should then process user arguments on whether sufficient inputs were given.

@bushwhackr
Copy link
Contributor Author

I'll wait for #3948

@bushwhackr bushwhackr closed this Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-madkins sfc-gh-madkins Awaiting requested review from sfc-gh-madkins

@niklasvm niklasvm Awaiting requested review from niklasvm

@kevjumba kevjumba Awaiting requested review from kevjumba

@yelo-blood yelo-blood Awaiting requested review from yelo-blood

@sudohainguyen sudohainguyen Awaiting requested review from sudohainguyen sudohainguyen is a code owner

Assignees
No one assigned
Labels
ok-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bushwhackr @sudohainguyen