Skip to content

Commit

Permalink
[ML] fix(next-pylint): Resolve arguments-differ and name-too-long (
Browse files Browse the repository at this point in the history
…Azure#31234)

* fix(next-pylint): Resolve `arguments-differ`

    Endpoint._from_rest_object:

        * Wasn't marked as @classmethod
        * Had a first argument of `self` instead of `cls`

    Made name of second parameter consistent across
    {Endpoint,BatchEndpoint,OnlineEndpoint}._from_rest_object

* refactor: validate_pipeline_input_key_contains_allowed_characters -> validate_pipeline_input_key_characters

* refactor: try_get_non_arbitrary_attr_for_potential_attr_dict -> try_get_non_arbitrary_attr

* refactor: parse_pipeline_input_names_from_data_binding -> PipelineExpression.parse_pipeline_inputs_from_data_binding

* refactor: get_choice_{,and_single_value_}schema_of_type -> choice_{,and_single_value_}schema_of_type

* fix: Add `pylint: disable=name-too-long`

    Added in situations where the isn't an obvious rename

* refactor: read_remote_feature_set_spec_metadata_contents -> read_remote_feature_set_spec_metadata

* refactor: Remove dead code
  • Loading branch information
kdestin authored Aug 2, 2023
1 parent d23f758 commit 382c742
Show file tree
Hide file tree
Showing 22 changed files with 100 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def items(self):
return result


class AISuperComputerStorageReferenceConfiguration(PascalCaseProperty):
class AISuperComputerStorageReferenceConfiguration(PascalCaseProperty): # pylint: disable=name-too-long
_KEY_MAPPING = {
"container_name": "ContainerName",
"relative_path": "RelativePath",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

module_logger = logging.getLogger(__name__)


# pylint: disable-next=name-too-long
class BatchPipelineComponentDeploymentConfiguarationsSchema(metaclass=PatchedSchemaMeta):
component_id = fields.Str()
job = UnionField(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from .source_metadata_schema import SourceMetadataSchema


# pylint: disable-next=name-too-long
class FeatureTransformationCodePropertiesSchema(metaclass=PatchedSchemaMeta):
path = fields.Str(data_key="Path")
transformer_class = fields.Str(data_key="TransformerClass")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def _add_data_binding_to_field(field, attrs_to_skip, schema_stack):
return field


def support_data_binding_expression_for_fields(
def support_data_binding_expression_for_fields( # pylint: disable=name-too-long
schema: Union[PathAwareSchema, Schema], attrs_to_skip=None, schema_stack=None
):
"""Update fields inside schema to support data binding string.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,26 @@
from azure.ai.ml._utils.utils import camel_to_snake


def get_choice_schema_of_type(cls, **kwargs):
def choice_schema_of_type(cls, **kwargs):
class CustomChoiceSchema(ChoiceSchema):
values = fields.List(cls(**kwargs))

return CustomChoiceSchema()


def get_choice_and_single_value_schema_of_type(cls, **kwargs):
def choice_and_single_value_schema_of_type(cls, **kwargs):
# Reshuffling the order of fields for allowing choice of booleans.
# The reason is, while dumping [Bool, Choice[Bool]] is parsing even dict as True.
# Since all unionFields are parsed sequentially, to avoid this, we are giving the "type" field at the end.
return UnionField([NestedField(get_choice_schema_of_type(cls, **kwargs)), cls(**kwargs)])
return UnionField([NestedField(choice_schema_of_type(cls, **kwargs)), cls(**kwargs)])


FLOAT_SEARCH_SPACE_DISTRIBUTION_FIELD = UnionField(
[
fields.Float(),
DumpableIntegerField(strict=True),
NestedField(get_choice_schema_of_type(DumpableIntegerField, strict=True)),
NestedField(get_choice_schema_of_type(fields.Float)),
NestedField(choice_schema_of_type(DumpableIntegerField, strict=True)),
NestedField(choice_schema_of_type(fields.Float)),
NestedField(UniformSchema()),
NestedField(QUniformSchema()),
NestedField(NormalSchema()),
Expand All @@ -64,15 +64,15 @@ def get_choice_and_single_value_schema_of_type(cls, **kwargs):
INT_SEARCH_SPACE_DISTRIBUTION_FIELD = UnionField(
[
DumpableIntegerField(strict=True),
NestedField(get_choice_schema_of_type(DumpableIntegerField, strict=True)),
NestedField(choice_schema_of_type(DumpableIntegerField, strict=True)),
NestedField(RandintSchema()),
NestedField(IntegerQUniformSchema()),
NestedField(IntegerQNormalSchema()),
]
)

STRING_SEARCH_SPACE_DISTRIBUTION_FIELD = get_choice_and_single_value_schema_of_type(DumpableStringField)
BOOL_SEARCH_SPACE_DISTRIBUTION_FIELD = get_choice_and_single_value_schema_of_type(fields.Bool)
STRING_SEARCH_SPACE_DISTRIBUTION_FIELD = choice_and_single_value_schema_of_type(DumpableStringField)
BOOL_SEARCH_SPACE_DISTRIBUTION_FIELD = choice_and_single_value_schema_of_type(fields.Bool)

model_size_enum_args = {"allowed_values": [o.value for o in ModelSize], "casing_transform": camel_to_snake}
learning_rate_scheduler_enum_args = {
Expand All @@ -86,14 +86,12 @@ def get_choice_and_single_value_schema_of_type(cls, **kwargs):
}


MODEL_SIZE_DISTRIBUTION_FIELD = get_choice_and_single_value_schema_of_type(
StringTransformedEnum, **model_size_enum_args
)
LEARNING_RATE_SCHEDULER_DISTRIBUTION_FIELD = get_choice_and_single_value_schema_of_type(
MODEL_SIZE_DISTRIBUTION_FIELD = choice_and_single_value_schema_of_type(StringTransformedEnum, **model_size_enum_args)
LEARNING_RATE_SCHEDULER_DISTRIBUTION_FIELD = choice_and_single_value_schema_of_type(
StringTransformedEnum, **learning_rate_scheduler_enum_args
)
OPTIMIZER_DISTRIBUTION_FIELD = get_choice_and_single_value_schema_of_type(StringTransformedEnum, **optimizer_enum_args)
VALIDATION_METRIC_DISTRIBUTION_FIELD = get_choice_and_single_value_schema_of_type(
OPTIMIZER_DISTRIBUTION_FIELD = choice_and_single_value_schema_of_type(StringTransformedEnum, **optimizer_enum_args)
VALIDATION_METRIC_DISTRIBUTION_FIELD = choice_and_single_value_schema_of_type(
StringTransformedEnum, **validation_metric_enum_args
)

Expand Down Expand Up @@ -128,6 +126,7 @@ class ImageModelDistributionSettingsSchema(metaclass=PatchedSchemaMeta):
weight_decay = FLOAT_SEARCH_SPACE_DISTRIBUTION_FIELD


# pylint: disable-next=name-too-long
class ImageModelDistributionSettingsClassificationSchema(ImageModelDistributionSettingsSchema):
model_name = STRING_SEARCH_SPACE_DISTRIBUTION_FIELD
training_crop_size = INT_SEARCH_SPACE_DISTRIBUTION_FIELD
Expand Down Expand Up @@ -163,6 +162,7 @@ def make(self, data, **kwargs):
return ImageClassificationSearchSpace(**data)


# pylint: disable-next=name-too-long
class ImageModelDistributionSettingsDetectionCommonSchema(ImageModelDistributionSettingsSchema):
box_detections_per_image = INT_SEARCH_SPACE_DISTRIBUTION_FIELD
box_score_threshold = FLOAT_SEARCH_SPACE_DISTRIBUTION_FIELD
Expand Down Expand Up @@ -206,9 +206,11 @@ def make(self, data, **kwargs):
return ImageObjectDetectionSearchSpace(**data)


# pylint: disable-next=name-too-long
class ImageModelDistributionSettingsObjectDetectionSchema(ImageModelDistributionSettingsDetectionCommonSchema):
model_name = STRING_SEARCH_SPACE_DISTRIBUTION_FIELD


# pylint: disable-next=name-too-long
class ImageModelDistributionSettingsInstanceSegmentationSchema(ImageModelDistributionSettingsObjectDetectionSchema):
model_name = STRING_SEARCH_SPACE_DISTRIBUTION_FIELD
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,23 @@
from azure.ai.ml._utils.utils import camel_to_snake


def get_choice_schema_of_type(cls, **kwargs):
def choice_schema_of_type(cls, **kwargs):
class CustomChoiceSchema(ChoiceSchema):
values = fields.List(cls(**kwargs))

return CustomChoiceSchema()


def get_choice_and_single_value_schema_of_type(cls, **kwargs):
return UnionField([cls(**kwargs), NestedField(get_choice_schema_of_type(cls, **kwargs))])
def choice_and_single_value_schema_of_type(cls, **kwargs):
return UnionField([cls(**kwargs), NestedField(choice_schema_of_type(cls, **kwargs))])


FLOAT_SEARCH_SPACE_DISTRIBUTION_FIELD = UnionField(
[
fields.Float(),
DumpableIntegerField(strict=True),
NestedField(get_choice_schema_of_type(DumpableIntegerField, strict=True)),
NestedField(get_choice_schema_of_type(fields.Float)),
NestedField(choice_schema_of_type(DumpableIntegerField, strict=True)),
NestedField(choice_schema_of_type(fields.Float)),
NestedField(UniformSchema()),
NestedField(QUniformSchema()),
NestedField(NormalSchema()),
Expand All @@ -54,19 +54,19 @@ def get_choice_and_single_value_schema_of_type(cls, **kwargs):
INT_SEARCH_SPACE_DISTRIBUTION_FIELD = UnionField(
[
DumpableIntegerField(strict=True),
NestedField(get_choice_schema_of_type(DumpableIntegerField, strict=True)),
NestedField(choice_schema_of_type(DumpableIntegerField, strict=True)),
NestedField(RandintSchema()),
]
)

STRING_SEARCH_SPACE_DISTRIBUTION_FIELD = get_choice_and_single_value_schema_of_type(DumpableStringField)
BOOL_SEARCH_SPACE_DISTRIBUTION_FIELD = get_choice_and_single_value_schema_of_type(fields.Bool)
STRING_SEARCH_SPACE_DISTRIBUTION_FIELD = choice_and_single_value_schema_of_type(DumpableStringField)
BOOL_SEARCH_SPACE_DISTRIBUTION_FIELD = choice_and_single_value_schema_of_type(fields.Bool)


class NlpParameterSubspaceSchema(metaclass=PatchedSchemaMeta):
gradient_accumulation_steps = INT_SEARCH_SPACE_DISTRIBUTION_FIELD
learning_rate = FLOAT_SEARCH_SPACE_DISTRIBUTION_FIELD
learning_rate_scheduler = get_choice_and_single_value_schema_of_type(
learning_rate_scheduler = choice_and_single_value_schema_of_type(
StringTransformedEnum,
allowed_values=[obj.value for obj in NlpLearningRateScheduler],
casing_transform=camel_to_snake,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ def make(self, data, **kwargs):
)


# pylint: disable-next=name-too-long
class AnonymousDataTransferImportComponentSchema(AnonymousAssetSchema, DataTransferImportComponentSchema):
"""Anonymous data transfer import component schema.
Expand All @@ -179,6 +180,7 @@ def make(self, data, **kwargs):
)


# pylint: disable-next=name-too-long
class AnonymousDataTransferExportComponentSchema(AnonymousAssetSchema, DataTransferExportComponentSchema):
"""Anonymous data transfer export component schema.
Expand Down
6 changes: 3 additions & 3 deletions sdk/ml/azure-ai-ml/azure/ai/ml/_schema/core/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
RESOURCE_ID_FORMAT,
AzureMLResourceType,
)
from ...entities._job.pipeline._attr_dict import try_get_non_arbitrary_attr_for_potential_attr_dict
from ...entities._job.pipeline._attr_dict import try_get_non_arbitrary_attr
from ...exceptions import ValidationException
from ..core.schema import PathAwareSchema

Expand Down Expand Up @@ -572,7 +572,7 @@ def _simplified_error_base_on_type(self, e, value, attr) -> Exception:
* First Matched Error message if value has type and type matches atleast one field
:rtype: Exception
"""
value_type = try_get_non_arbitrary_attr_for_potential_attr_dict(value, self.type_field_name)
value_type = try_get_non_arbitrary_attr(value, self.type_field_name)
if value_type is None:
# if value has no type field, raise original error
return e
Expand All @@ -596,7 +596,7 @@ def _simplified_error_base_on_type(self, e, value, attr) -> Exception:

def _serialize(self, value, attr, obj, **kwargs):
union_fields = self._union_fields[:]
value_type = try_get_non_arbitrary_attr_for_potential_attr_dict(value, self.type_field_name)
value_type = try_get_non_arbitrary_attr(value, self.type_field_name)
if value_type is not None and value_type in self.allowed_types:
target_fields = self._type_sensitive_fields_dict[value_type]
if len(target_fields) == 1:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def make(self, data, **kwargs):
return PredictionDriftMetricThreshold(**data)


# pylint: disable-next=name-too-long
class FeatureAttributionDriftMetricThresholdSchema(MetricThresholdSchema):
@post_load
def make(self, data, **kwargs):
Expand Down
4 changes: 2 additions & 2 deletions sdk/ml/azure-ai-ml/azure/ai/ml/_utils/_feature_store_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
from azure.ai.ml.operations._feature_store_entity_operations import FeatureStoreEntityOperations


def read_feature_set_metadata_contents(*, path: str) -> Dict:
def read_feature_set_metadata(*, path: str) -> Dict:
metadata_path = str(Path(path, "FeatureSetSpec.yaml"))
return load_yaml(metadata_path)


def read_remote_feature_set_spec_metadata_contents(
def read_remote_feature_set_spec_metadata(
*,
base_uri: str,
datastore_operations: DatastoreOperations,
Expand Down
8 changes: 1 addition & 7 deletions sdk/ml/azure-ai-ml/azure/ai/ml/_utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,12 +407,6 @@ def dict_eq(dict1: Dict[str, Any], dict2: Dict[str, Any]) -> bool:
return dict1 == dict2


def get_http_response_and_deserialized_from_pipeline_response(
pipeline_response: Any, deserialized: Any
) -> Tuple[Any, Any]:
return pipeline_response.http_response, deserialized


def xor(a: Any, b: Any) -> bool:
return bool(a) != bool(b)

Expand Down Expand Up @@ -765,7 +759,7 @@ def is_on_disk_cache_enabled():
return os.getenv(AZUREML_DISABLE_ON_DISK_CACHE_ENV_VAR) not in ["True", "true", True]


def is_concurrent_component_registration_enabled():
def is_concurrent_component_registration_enabled(): # pylint: disable=name-too-long
return os.getenv(AZUREML_DISABLE_CONCURRENT_COMPONENT_REGISTRATION) not in ["True", "true", True]


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from azure.ai.ml.entities._job.automl.automl_job import AutoMLJob
from azure.ai.ml.entities._job.pipeline._attr_dict import (
has_attr_safe,
try_get_non_arbitrary_attr_for_potential_attr_dict,
try_get_non_arbitrary_attr,
)
from azure.ai.ml.entities._job.pipeline._pipeline_expression import PipelineExpression
from azure.ai.ml.entities._validation import MutableValidationResult
Expand Down Expand Up @@ -210,7 +210,7 @@ def _get_input_binding_dict(self, node: BaseNode) -> Tuple[dict, dict]:
component_binding_input = component_binding_input.path
if is_data_binding_expression(component_binding_input, ["parent"]):
# data binding may have more than one PipelineInput now
for pipeline_input_name in PipelineExpression.parse_pipeline_input_names_from_data_binding(
for pipeline_input_name in PipelineExpression.parse_pipeline_inputs_from_data_binding(
component_binding_input
):
if pipeline_input_name not in self.inputs:
Expand Down Expand Up @@ -370,11 +370,7 @@ def _check_ignored_keys(cls, obj):
"settings": lambda val: val is not None and any(v is not None for v in val._to_dict().values()),
}
# Avoid new attr added by use `try_get_non...` instead of `hasattr` or `getattr` directly.
return [
k
for k, has_set in examine_mapping.items()
if has_set(try_get_non_arbitrary_attr_for_potential_attr_dict(obj, k))
]
return [k for k, has_set in examine_mapping.items() if has_set(try_get_non_arbitrary_attr(obj, k))]

def _get_telemetry_values(self, *args, **kwargs):
telemetry_values = super()._get_telemetry_values()
Expand Down
24 changes: 12 additions & 12 deletions sdk/ml/azure-ai-ml/azure/ai/ml/entities/_endpoint/batch_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,19 +89,19 @@ def _to_rest_batch_endpoint(self, location: str) -> BatchEndpointData:
return BatchEndpointData(location=location, tags=self.tags, properties=batch_endpoint)

@classmethod
def _from_rest_object(cls, endpoint: BatchEndpointData): # pylint: disable=arguments-renamed
def _from_rest_object(cls, obj: BatchEndpointData) -> "BatchEndpoint":
return BatchEndpoint(
id=endpoint.id,
name=endpoint.name,
tags=endpoint.tags,
properties=endpoint.properties.properties,
auth_mode=camel_to_snake(endpoint.properties.auth_mode),
description=endpoint.properties.description,
location=endpoint.location,
defaults=endpoint.properties.defaults,
provisioning_state=endpoint.properties.provisioning_state,
scoring_uri=endpoint.properties.scoring_uri,
openapi_uri=endpoint.properties.swagger_uri,
id=obj.id,
name=obj.name,
tags=obj.tags,
properties=obj.properties.properties,
auth_mode=camel_to_snake(obj.properties.auth_mode),
description=obj.properties.description,
location=obj.location,
defaults=obj.properties.defaults,
provisioning_state=obj.properties.provisioning_state,
scoring_uri=obj.properties.scoring_uri,
openapi_uri=obj.properties.swagger_uri,
)

def dump(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ def provisioning_state(self) -> Optional[str]:
def dump(self, dest: Optional[Union[str, PathLike, IO[AnyStr]]] = None, **kwargs) -> None:
pass

@classmethod
@abstractmethod
def _from_rest_object(self, obj: Any) -> Any:
def _from_rest_object(cls, obj: Any) -> Any:
pass

def _merge_with(self, other: "Endpoint") -> None:
Expand Down
Loading

0 comments on commit 382c742

Please sign in to comment.