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

Fix _add_enum_value_python_name() to respect explicitly set value names #2073

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+301 −8
Open

Fix _add_enum_value_python_name() to respect explicitly set value names #2073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bdaa4ad
Add failing enum value build tests
ni-jfitzger Jan 26, 2025
877be66
Fix _add_enum_value_python_name
ni-jfitzger Jan 26, 2025
64f00ea
Override some metadata to prevent API changes
ni-jfitzger Jan 26, 2025
bf0540b
Rewrite change to use a temporary key, '_python_name', for processing.
ni-jfitzger Jan 26, 2025
ff725c9
Tweak: don't skip prefix and suffix removal just because there's only…
ni-jfitzger Jan 26, 2025
701c364
No need to use python_name for last remaining value
ni-jfitzger Jan 26, 2025
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
35 changes: 27 additions & 8 deletions build/helper/metadata_add_all.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,12 +609,15 @@ def _get_least_restrictive_codegen_method(codegen_methods):
def _add_enum_value_python_name(enum_info, config):
'''Add 'python_name' for all values, removing any common prefixes and suffixes'''
for v in enum_info['values']:
# Some values have an explicitly set python_name.
# To avoid altering these, we'll use _python_name for all of our processing.
if 'python_name' not in v:
v['python_name'] = v['name'].replace('{}_VAL_'.format(config['module_name'].upper()), '')
v['_python_name'] = v['name'].replace('{}_VAL_'.format(config['module_name'].upper()), '')

# We are using an os.path function do find any common prefix. So that we don't
# We are using an os.path function to find any common prefix. So that we don't
# get 'O' in 'ON' and 'OFF' we remove characters at the end until they are '_'
names = [v['python_name'] for v in enum_info['values']]
# Exclude explicitly set names from the prefix calculation
names = [v['_python_name'] for v in enum_info['values'] if '_python_name' in v]
prefix = os.path.commonprefix(names)
while len(prefix) > 0 and prefix[-1] != '_':
prefix = prefix[:-1]
Expand All @@ -627,13 +630,20 @@ def _add_enum_value_python_name(enum_info, config):
# '_' only means the name starts with a number
if len(prefix) > 0 and prefix != '_':
for v in enum_info['values']:
assert v['python_name'].startswith(prefix), '{} does not start with {}'.format(v['name'], prefix)
if 'python_name' in v:
continue
assert v['_python_name'].startswith(prefix), '{} does not start with {}'.format(v['name'], prefix)
v['prefix'] = prefix
v['python_name'] = v['python_name'].replace(prefix, '')
v['_python_name'] = v['_python_name'].replace(prefix, '')

# Now we need to look for common suffixes
# Using the slow method of reversing a string for readability
rev_names = [''.join(reversed(v['python_name'])) for v in enum_info['values']]
# We exclude explicitly set names when looking for common suffixes
rev_names = [
''.join(reversed(v['_python_name']))
for v in enum_info['values']
if '_python_name' in v
]
suffix = os.path.commonprefix(rev_names)
while len(suffix) > 0 and suffix[-1] != '_':
suffix = suffix[:-1]
Expand All @@ -649,12 +659,21 @@ def _add_enum_value_python_name(enum_info, config):
# '_' only means the name starts with a number
if len(suffix) > 0:
for v in enum_info['values']:
assert v['python_name'].endswith(suffix), '{} does not end with {}'.format(v['name'], suffix)
if 'python_name' in v:
continue
assert v['_python_name'].endswith(suffix), '{} does not end with {}'.format(v['name'], suffix)
v['suffix'] = suffix
v['python_name'] = v['python_name'][:-len(suffix)]
v['_python_name'] = v['_python_name'][:-len(suffix)]

for v in enum_info['values']:
if 'python_name' in v:
continue
v['python_name'] = v['_python_name']
del v['_python_name']

# We need to check again to see if we have any values that start with a digit
# If we are not going to code generate this enum, we don't care about this
# All names should follow this rule
for v in enum_info['values']:
assert v['python_name'], enum_info
if enum_info['codegen_method'] != 'no' and v['python_name'][0].isdigit():
Expand Down
113 changes: 113 additions & 0 deletions build/unit_tests/test_metadata_add_all.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,76 @@ def _compare_dicts(actual, expected):
}
]
},
'EnumWithCommonPrefixInValueNames': {
'codegen_method': 'public',
'values': [
{
'name': 'COLOR_BRIGHT_RED',
'value': 1
},
{
'name': 'COLOR_BRIGHT_BLUE',
'value': 2
}
]
},
'EnumWithExplicitValueNames': {
'codegen_method': 'public',
'values': [
{
'name': 'THE_COLOR_RED',
'python_name': 'COLOR_DARK_RED',
'value': 1
},
{
'name': 'THE_COLOR_BLUE',
'python_name': 'COLOR_DARK_BLUE',
'value': 2
}
]
},
'EnumWithExplicitValueNamesMixedIn': {
'codegen_method': 'public',
'values': [
{
'name': 'DISTANCE_MILES',
'value': 1
},
{
'name': 'DISTANCE_KILOMETERS',
'python_name': 'DISTANCE_KILOMETERS',
'value': 2
},
{
'name': 'DISTANCE_METERS',
'python_name': 'DISTANCE_METERS',
'value': 5
},
{
'name': 'DISTANCE_YARDS',
'value': 42
}
]
},
'EnumWithOneExpandedValueAndCommonPrefixSuffix': {
'codegen_method': 'public',
'values': [
{
'name': 'COMMON_PREFIX_FOOTBALL_COMMON_SUFFIX',
'value': 1
},
{
'name': 'COMMON_PREFIX_BASEBALL',
'python_name': 'BASEBALL', # we want to test that this excludes it from common prefix/suffix calculations
'value': 2
},
{
'name': 'BASKETBALL_COMMON_SUFFIX',
'python_name': 'BASKETBALL', # we want to test that this excludes it from common prefix/suffix calculations
'value': 3
},
]
},
}


Expand All @@ -896,6 +966,41 @@ def _compare_dicts(actual, expected):
{'name': 'BLACK', 'value': 42, 'converts_to_value': 42, 'python_name': 'BLACK'}
]
},
'EnumWithCommonPrefixInValueNames': {
'codegen_method': 'public',
'python_name': 'EnumWithCommonPrefixInValueNames',
'values': [
{'name': 'COLOR_BRIGHT_RED', 'value': 1, 'python_name': 'RED', 'prefix': 'COLOR_BRIGHT_'},
{'name': 'COLOR_BRIGHT_BLUE', 'value': 2, 'python_name': 'BLUE', 'prefix': 'COLOR_BRIGHT_'}
]
},
'EnumWithExplicitValueNames': {
'codegen_method': 'public',
'python_name': 'EnumWithExplicitValueNames',
'values': [
{'name': 'THE_COLOR_RED', 'value': 1, 'python_name': 'COLOR_DARK_RED'},
{'name': 'THE_COLOR_BLUE', 'value': 2, 'python_name': 'COLOR_DARK_BLUE'}
]
},
'EnumWithExplicitValueNamesMixedIn': {
'codegen_method': 'public',
'python_name': 'EnumWithExplicitValueNamesMixedIn',
'values': [
{'name': 'DISTANCE_MILES', 'value': 1, 'python_name': 'MILES', 'prefix': 'DISTANCE_'},
{'name': 'DISTANCE_KILOMETERS', 'value': 2, 'python_name': 'DISTANCE_KILOMETERS'}, # explicitly set
{'name': 'DISTANCE_METERS', 'value': 5, 'python_name': 'DISTANCE_METERS'}, # explicitly set
{'name': 'DISTANCE_YARDS', 'value': 42, 'python_name': 'YARDS', 'prefix': 'DISTANCE_'}
]
},
'EnumWithOneExpandedValueAndCommonPrefixSuffix': {
'codegen_method': 'public',
'python_name': 'EnumWithOneExpandedValueAndCommonPrefixSuffix',
'values': [
{'name': 'COMMON_PREFIX_FOOTBALL_COMMON_SUFFIX', 'value': 1, 'python_name': 'SUFFIX', 'prefix': 'COMMON_PREFIX_FOOTBALL_COMMON_'},
{'name': 'COMMON_PREFIX_BASEBALL', 'value': 2, 'python_name': 'BASEBALL'}, # explicitly set
{'name': 'BASKETBALL_COMMON_SUFFIX', 'value': 3, 'python_name': 'BASKETBALL'}, # explicitly set
]
},
}


Expand Down Expand Up @@ -1113,6 +1218,10 @@ def test_get_functions_that_use_enums():
expected_output = {
'Color': ['PythonOnlyMethod'],
'EnumWithConverter': ['PublicMethod', 'PrivateMethod'],
'EnumWithCommonPrefixInValueNames': [],
'EnumWithExplicitValueNames': [],
'EnumWithExplicitValueNamesMixedIn': [],
'EnumWithOneExpandedValueAndCommonPrefixSuffix': [],
}
actual_output = _get_functions_that_use_enums(actual_enums, actual_config)
_compare_dicts(actual_output, expected_output)
Expand All @@ -1123,6 +1232,10 @@ def test_get_attributes_that_use_enums():
expected_output = {
'Color': ['1000002'],
'EnumWithConverter': ['1000001', '1000003'],
'EnumWithCommonPrefixInValueNames': [],
'EnumWithExplicitValueNames': [],
'EnumWithExplicitValueNamesMixedIn': [],
'EnumWithOneExpandedValueAndCommonPrefixSuffix': [],
}
actual_output = _get_attributes_that_use_enums(actual_enums, actual_config)
_compare_dicts(actual_output, expected_output)
Expand Down
125 changes: 125 additions & 0 deletions src/nidcpower/metadata/enums_addon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,130 @@
# Any changes to the API should be made here. enums.py is code generated

enums_override_metadata = {
# TODO (ni-jfitzger): delete this override once python_name is corrected for each value. See https://github.com/ni/nimi-python/issues/2072
'ComplianceLimitSymmetry': {
'values': [
{
'documentation': {
'description': 'Compliance limits are specified symmetrically about 0.'
},
'name': 'NIDCPOWER_VAL_COMPLIANCE_LIMIT_SYMMETRY_SYMMETRIC',
'value': 0
},
{
'documentation': {
'description': 'Compliance limits can be specified asymmetrically with respect to 0.'
},
'name': 'NIDCPOWER_VAL_COMPLIANCE_LIMIT_SYMMETRY_ASYMMETRIC',
'value': 1
}
]
},
# TODO (ni-jfitzger): delete this override once python_name is corrected for each value. See https://github.com/ni/nimi-python/issues/2072
'Event': {
'values': [
{
'documentation': {
'description': 'Specifies the Source Complete event.'
},
'name': 'NIDCPOWER_VAL_SOURCE_COMPLETE_EVENT',
'python_name': 'SOURCE_COMPLETE',
'value': 1030
},
{
'documentation': {
'description': 'Specifies the Measure Complete event.'
},
'name': 'NIDCPOWER_VAL_MEASURE_COMPLETE_EVENT',
'python_name': 'MEASURE_COMPLETE',
'value': 1031
},
{
'documentation': {
'description': 'Specifies the Sequence Iteration Complete event.'
},
'name': 'NIDCPOWER_VAL_SEQUENCE_ITERATION_COMPLETE_EVENT',
'python_name': 'SEQUENCE_ITERATION_COMPLETE',
'value': 1032
},
{
'documentation': {
'description': 'Specifies the Sequence Engine Done event.'
},
'name': 'NIDCPOWER_VAL_SEQUENCE_ENGINE_DONE_EVENT',
'python_name': 'SEQUENCE_ENGINE_DONE',
'value': 1033
},
{
'documentation': {
'description': 'Specifies the Pulse Complete event.'
},
'name': 'NIDCPOWER_VAL_PULSE_COMPLETE_EVENT',
'python_name': 'PULSE_COMPLETE',
'value': 1051
},
{
'documentation': {
'description': 'Specifies the Ready for Pulse Trigger event.'
},
'name': 'NIDCPOWER_VAL_READY_FOR_PULSE_TRIGGER_EVENT',
'python_name': 'READY_FOR_PULSE_TRIGGER',
'value': 1052
}
]
},
# TODO (ni-jfitzger): delete this override once python_name is corrected for each value. See https://github.com/ni/nimi-python/issues/2072
'SendSoftwareEdgeTriggerType': {
'values': [
{
'documentation': {
'description': 'Asserts the Start trigger.'
},
'name': 'NIDCPOWER_VAL_START_TRIGGER',
'python_name': 'START',
'value': 1034
},
{
'documentation': {
'description': 'Asserts the Source trigger.'
},
'name': 'NIDCPOWER_VAL_SOURCE_TRIGGER',
'python_name': 'SOURCE',
'value': 1035
},
{
'documentation': {
'description': 'Asserts the Measure trigger.'
},
'name': 'NIDCPOWER_VAL_MEASURE_TRIGGER',
'python_name': 'MEASURE',
'value': 1036
},
{
'documentation': {
'description': 'Asserts the Sequence Advance trigger.'
},
'name': 'NIDCPOWER_VAL_SEQUENCE_ADVANCE_TRIGGER',
'python_name': 'SEQUENCE_ADVANCE',
'value': 1037
},
{
'documentation': {
'description': 'Asserts the Pulse trigger.'
},
'name': 'NIDCPOWER_VAL_PULSE_TRIGGER',
'python_name': 'PULSE',
'value': 1053
},
{
'documentation': {
'description': 'Asserts the Shutdown trigger.'
},
'name': 'NIDCPOWER_VAL_SHUTDOWN_TRIGGER',
'python_name': 'SHUTDOWN',
'value': 1118
}
]
},
}

36 changes: 36 additions & 0 deletions src/nidmm/metadata/enums_addon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,41 @@
# Any changes to the API should be made here. enums.py is code generated

enums_override_metadata = {
# TODO (ni-jfitzger): delete this override once python_name is corrected for each value. See https://github.com/ni/nimi-python/issues/2072
'ThermistorType': {
'values': [
{
'documentation': {
'description': 'Custom'
},
'name': 'NIDMM_VAL_TEMP_THERMISTOR_CUSTOM',
'value': 0
},
{
'documentation': {
'description': '44004'
},
'name': 'NIDMM_VAL_TEMP_THERMISTOR_44004',
'python_name': 'THERMISTOR_44004',
'value': 1
},
{
'documentation': {
'description': '44006'
},
'name': 'NIDMM_VAL_TEMP_THERMISTOR_44006',
'python_name': 'THERMISTOR_44006',
'value': 2
},
{
'documentation': {
'description': '44007'
},
'name': 'NIDMM_VAL_TEMP_THERMISTOR_44007',
'python_name': 'THERMISTOR_44007',
'value': 3
}
]
},
}