Fixing size parameter handling in len-mechanism to handle function parameters with different size parameters#2138
Fixing size parameter handling in len-mechanism to handle function parameters with different size parameters#2138
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2138 +/- ##
==========================================
+ Coverage 90.09% 90.12% +0.03%
==========================================
Files 71 71
Lines 18662 18675 +13
==========================================
+ Hits 16813 16831 +18
+ Misses 1849 1844 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
It seems we've gone from using flake8 7.1.2 to using flake8 7.3.0 and the new one has this new error code. https://docs.python.org/3/reference/simple_stmts.html#the-global-statement This seems like something we should address in a separate PR. |
You only need the https://stackoverflow.com/questions/291978/short-description-of-the-scoping-rules |
I have added a |
build/templates/_library_interpreter.py/initialization_method.py.mako
Outdated
Show resolved
Hide resolved
|
|
||
| def test_filter_parameters_mixed_usage_ivi_dance_and_len(): | ||
| parameters = [ | ||
| _parameter('ivi_size', mechanism='passed-in'), |
There was a problem hiding this comment.
From looking at nifake/functions.py, the mechanism isn't typically set for and ivi-dance size parameter or a length size parameter. In fact, not even size is set. So, what seems to end up happening is that size gets added for each by metadata_add_all._add_buffer_info with a "mechanism" of "fixed" and a "value" of 1. test_metadata_add_all shows further evidence of this.
I don't think it's important to set mechanism for 'ivi_size' or 'len_size' in this test, but if you're going to explicitly set them, you should probably set them correctly.
There was a problem hiding this comment.
As to why it's fixed when calculate it in the library interpreter?
I think it's just an usused default value, but I haven't traced the usage, myself.
| _parameter('out_waveform', direction='out', mechanism='ivi-dance', size_value='ivi_size'), | ||
| _parameter('len_size', mechanism='passed-in'), | ||
| _parameter('len_data', mechanism='len', size_value='len_size'), | ||
| _parameter('timeout', mechanism='fixed'), |
There was a problem hiding this comment.
If you're going to set mechanism to 'fixed' for a parameter, you set size_value to a number and direction to 'out'. If you want it to be a parameter that gets passed in, the mechanism should be 'passed-in'.
I've updated CHANGELOG.md if applicable.What does this Pull Request accomplish?
Fixes the codegen to properly handle functions with multiple array parameters that reference different size parameters:
Updates
metadata_filters.py: Refactors thefilter_parametersfunction to collect all size parameter names referenced by len-sized parameters into alen_size_parameter_names, replacing the previous singlelen_size_parameterapproach. Removed assert to check if all size parameters are same fromfilter_len_parameters. Following this updated var names indefault_method.py.makoandinitialization_method.py.makoto reflect multiple possible size params.Updates to nifake to enable testing: Details covered in testing section
Previously, it was assumed all len-mechanism arrays in a function shared the same size parameter. This change enables functions like
CreateDeembeddingSparameterTableArrayin NI-RFSG wherefrequenciesusesfrequenciesSizeandsparameterTableusessparameterTableSize.List issues fixed by this Pull Request
Issue 2137: Fix size parameter handling for functions with parameters referencing multiple size parameters.
Additional PR needs to be raised to update NI-RFSG function metadata (CreateDeembeddingSparameterTableArray) to consume
these changes.
What testing has been done?
MultipleArraysDifferentSizefunction to nifake metadata with two arrays (valuesArrayusingvaluesArraySize, anddataArrayusingdataArraySize) to test the change in filter param logictest_multiple_arrays_different_sizeintest_library_interpreter.pythat verifies correct handling of multiple arrays with independent size parametersvaluesArraywithvaluesArraySize, and 5 elements fordataArraywithdataArraySize)