Error in return type for most 'autorange' properties in nidcpower library

[Arkh42]

Description of issue

In the code documentation, most of the "autorange"-related parameter are described as bool, which is wrong.
The correct type is int.

Steps to reproduce issue

``` python
import pytest

import nidcpower

@pytest.fixture
def smu() -> nidcpower.Session:
    return nidcpower.Session(
        resource_name="PXI1Slot2",
        channels=None,
        reset=True,
        options={'range_check': True,
                 'simulate': True,
                 'driver_setup': {'Model': '4163', 'BoardType': 'PXIe'}},
        independent_channels=True)

def test_wrong_type(smu):

    assert isinstance(smu.autorange, bool)

def test_correct_type(smu):

    assert isinstance(smu.autorange, int)
```

1. Run pytest on the above script.
2. The first test fails because an AssertiorError is raised.
3. The second test passes.

[marcoskirsch]

Hello,

In the code documentation, most of the "autorange"-related parameter are described as bool, which is wrong.
The correct type is int.

Looking at the nidcpower.Session code, I see that the type of property autorange is bool, which matches documentation.

You may be thinking bool is wrong because the NI-DCPower C API defines the attribute as ViInt32. But while our Python and C APIs are very close matches, they do deviate in small specific ways enabled by the nature of the Python language. This is an example of that. Please check our Python API Design Guidelines for more detail.

Thanks for the feedback!

[Arkh42]

Hello,

I do not think that bool is wrong because of the C API, but because Python tells me so. (See the pytest code example.)

The first link that you provided, which refers to nidcpower._SessionBase in the session module, proves that the documentation is wrong, as Python sees the returned type as an int.

[marcoskirsch]

I see, I misread the code.

However bool (as documented) is right.
Returning int is wrong, we don't use magic int values in the Python API. A property like this would be bool if possible, its own enum type if not.

[Arkh42]

It happens.
Thanks for considering it again. :)

[marcoskirsch]

most of the "autorange"-related parameter are described as bool, which is wrong.

Were there other properties in the API that seem wrong?

[Arkh42]

Here is a list of the properties that are documented as bool but are in fact int in Python:

• autorange
• current_limit_autorange
• current_level_autorange
• voltage_limit_autorange
• voltage_level_autorange

EDIT: I tested all of them in a similar fashion as the above test script provided when I opened the issue.

[ni-jfitzger]

Based on what @marcoskirsch has told me, we should have done something like this with an enum for each of these properties:

    'IsolationState': {
        'codegen_method': 'private',
        'converted_value_to_enum_function_name': 'convert_to_isolation_state_enum',
        'enum_to_converted_value_function_name': 'convert_from_isolation_state_enum',
        'values': [
            {
                'converts_to_value': True,
                'documentation': {
                    'description': 'The channel is disconnected from chassis ground.'
                },
                'name': 'NIDCPOWER_VAL_ISOLATED',
                'value': 1128
            },
            {
                'converts_to_value': False,
                'documentation': {
                    'description': 'The channel is connected to chassis ground.'
                },
                'name': 'NIDCPOWER_VAL_NON_ISOLATED',
                'value': 1129
            }
        ]
    },

Key points:

• Have a private enum that we convert to/from (in some or all cases that enum doesn't exist, even privately, for the Python API, I think, but does for the C API)
• For that enum:
  • define converted_value_to_enum_function_name
  • define enum_to_converted_value_function_name
  • define converts_to_value for each value

Before we make a change like this, we need to think through the implications and do some testing to confirm it's not a breaking change.