diff --git a/feathr_project/feathr/__init__.py b/feathr_project/feathr/__init__.py index 801c39b96..7e74ef90d 100644 --- a/feathr_project/feathr/__init__.py +++ b/feathr_project/feathr/__init__.py @@ -13,3 +13,4 @@ from .lookup_feature import LookupFeature from .aggregation import Aggregation from .feathr_configurations import SparkExecutionConfiguration +from .api.app.core.feathr_api_exception import * \ No newline at end of file diff --git a/feathr_project/feathr/api/app/core/feathr_api_exception.py b/feathr_project/feathr/api/app/core/feathr_api_exception.py index 0ced2340b..3c6be221e 100644 --- a/feathr_project/feathr/api/app/core/feathr_api_exception.py +++ b/feathr_project/feathr/api/app/core/feathr_api_exception.py @@ -1,8 +1,17 @@ -from app.core.error_code import ErrorCode - +from feathr.api.app.core.error_code import ErrorCode class FeathrApiException(Exception): def __init__(self,message:str,code:ErrorCode): self.code = code self.message = message + +class FeatureNameValidationError(ValueError): + """An exception for feature name validation. + Feature names must consist of letters, number, or underscores, + and cannot begin with a number. + Periods are also disallowed, as some compute engines, such as Spark, + will consider them as operators in feature name. + """ + + diff --git a/feathr_project/feathr/feature.py b/feathr_project/feathr/feature.py index ac4b2b774..b46585993 100644 --- a/feathr_project/feathr/feature.py +++ b/feathr_project/feathr/feature.py @@ -1,13 +1,15 @@ -from abc import ABC, abstractmethod +import re from copy import deepcopy from typing import List, Optional, Union, Dict from jinja2 import Template +from feathr.api.app.core.feathr_api_exception import FeatureNameValidationError from feathr.dtype import FeatureType -from feathr.transformation import ExpressionTransformation, Transformation, WindowAggTransformation -from feathr.typed_key import DUMMY_KEY, TypedKey from feathr.frameconfig import HoconConvertible +from feathr.transformation import ExpressionTransformation, Transformation +from feathr.typed_key import DUMMY_KEY, TypedKey + class FeatureBase(HoconConvertible): """The base class for features @@ -28,6 +30,7 @@ def __init__(self, key: Optional[Union[TypedKey, List[TypedKey]]] = [DUMMY_KEY], registry_tags: Optional[Dict[str, str]] = None, ): + FeatureBase.validate_feature_name(name) self.name = name self.feature_type = feature_type self.registry_tags=registry_tags @@ -45,6 +48,29 @@ def __init__(self, # self.key could be null, when getting features from registry. self.key_alias = [k.key_column_alias for k in self.key if k] + @classmethod + def validate_feature_name(cls, feature_name: str) -> bool: + """ + Only alphabet, numbers, and '_' are allowed in the name. + It can not start with numbers. Note that '.' is NOT ALLOWED! + This is because some compute engines, such as Spark, will consider them as operators in feature name. + """ + if not feature_name: + raise FeatureNameValidationError('Feature name rule violation: empty feature name detected') + + feature_validator = re.compile(r"""^ # from the start of the string + [a-zA-Z_]{1} # first character can only be a letter or underscore + [a-zA-Z0-9_]+ # as many letters, numbers, or underscores as you like + $""", # to the end of the string + re.X) + + if not feature_validator.match(feature_name): + raise FeatureNameValidationError( + 'Feature name rule violation: only letters, numbers, and underscores are allowed in the name, ' + + f'and the name cannot start with a number. name={feature_name}') + + return True + def with_key(self, key_alias: Union[str, List[str]]): """Rename the feature key with the alias. This is useful in derived features that depends on the same feature with different keys.""" diff --git a/feathr_project/test/test_feature_name_validation.py b/feathr_project/test/test_feature_name_validation.py new file mode 100644 index 000000000..fb0bff12e --- /dev/null +++ b/feathr_project/test/test_feature_name_validation.py @@ -0,0 +1,36 @@ +import string + +import pytest + +from feathr import FeatureBase, FeatureNameValidationError + + +@pytest.mark.parametrize('bad_feature_name', + [None, + '']) +def test_feature_name_fails_on_empty_name(bad_feature_name: str): + with pytest.raises(FeatureNameValidationError, match="empty feature name"): + FeatureBase.validate_feature_name(bad_feature_name) + + +def test_feature_name_fails_on_leading_number(): + with pytest.raises(FeatureNameValidationError, match="cannot start with a number"): + FeatureBase.validate_feature_name("4featurename") + + +def test_feature_name_fails_on_punctuation_chars(): + for char in set(string.punctuation) - set('_'): + with pytest.raises(FeatureNameValidationError, match="only letters, numbers, and underscores are allowed"): + FeatureBase.validate_feature_name(f"feature_{char}_name") + + +@pytest.mark.parametrize('feature_name', + ["feature_name", + "features4lyfe", + "f_4_feature_", + "_leading_underscores_are_ok", + "CapitalizedFeature" + '']) +def test_feature_name_validates_ok(feature_name: str): + assert FeatureBase.validate_feature_name(feature_name) +