Implemented skip for Tools.Layouts in test_constant_exists with older OpticStudio versions

[LucVV]

Proposed change

test_constant_exists currently tests all constants, including constants under Tools.Layouts. This fails on OpticStudio versions < 24R1 as Tools.Layouts did not exist yet. I think it is optimal to implement the skip within the tests in this instance, as the tested constants are outomatically determined.

Type of change

• Example (a notebook demonstrating how to use ZOSPy for a specific application)
• Bugfix (non-breaking change which fixes an issue)
• New analysis (a wrapper around an OpticStudio analysis)
• New feature (other than an analysis)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests
• Documentation (improvement of either the docstrings or the documentation website)

Additional information

• Python version: 3.11
• OpticStudio version: 20.3.2

Related issues

Checklist

• I have followed the contribution guidelines
• The code has been linted, formatted and tested locally using tox.
• Local tests pass. Please fix any problems before opening a PR. If this is not possible, specify what doesn't work and why you can't fix it.
• I added new tests for any features contributed, or updated existing tests.
• I updated CHANGELOG.md with my changes (except for refactorings and changes in the documentation).

If you contributed an analysis:

• I did not use AttrDicts for the analysis result data (please use dataclasses instead).

If you contributed an example:

• I contributed my example as a Jupyter notebook.

[crnh]

Thanks for fixing this! I expect we will see more constants being added in future releases, so we may consider a more generic solution (e.g. with a dictionary mapping constants to the version in which they were added). Do you think we should opt for a more generic implementation now, or should we wait until more constants are added?

[LucVV]

Thanks for fixing this! I expect we will see more constants being added in future releases, so we may consider a more generic solution (e.g. with a dictionary mapping constants to the version in which they were added). Do you think we should opt for a more generic implementation now, or should we wait until more constants are added?

It might be nice to directly make it future proof. I see two options:

MINIMAL_ZOS_VERSION_FOR_CONSTANT = {  # Supply as {"constant": "zos version string"}
    "Tools.Layouts": "24.1.0"
}

# ...

    def test_constant_exists(self, zos, annotation, optic_studio_version):  # noqa: ARG002
        for constant_name, minimal_optic_studio_version in MINIMAL_ZOS_VERSION_FOR_CONSTANT.items():
            if annotation.enum.startswith(constant_name) and optic_studio_version < minimal_optic_studio_version:
                pytest.skip(f"{annotation.enum} is only available from OpticStudio {minimal_optic_studio_version} onwards")

which is quite inefficient, but does the trick. Or

MINIMAL_ZOS_VERSION_FOR_CONSTANTS = {  # Supply as {"zos version string": constant}
    "24.1.0": ["Tools.Layouts"]
}

# ...
    
    def test_constant_exists(self, zos, annotation, optic_studio_version):  # noqa: ARG002
        for minimal_optic_studio_version, constant_names in MINIMAL_ZOS_VERSION_FOR_CONSTANTS.items():
            if optic_studio_version < minimal_optic_studio_version and any(annotation.enum.startswith(constant_name) for constant_name in constant_names):
                pytest.skip(f"{annotation.enum} is only available from OpticStudio {minimal_optic_studio_version} onwards")

Which might be slightly more efficient as we first evaluate if we need to check it for this optic_studio_version and only if so we loop through the constants that should be skipped. Was this what you had in mind?

[crnh]

I chose a slightly different implementation. @LucVV can you check if this works correctly in your OpticStudio version?

[LucVV]

Works great!