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.

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

Refactor or deprecate Wave_TestResourceMetrics tests that call environmental_contour #109

Open
simmsa opened this issue Dec 21, 2023 · 1 comment · Fixed by #147
Open
Assignees

Comments

@simmsa
Copy link
Contributor

simmsa commented Dec 21, 2023

Description

test_environmental_contour, test_plot_environmental, and test_plot_environmental_contour_multiyear tests are failing. This issue initiates discussions on whether to refactor or deprecate these tests.

Context

  • Failing Tests: test_environmental_contour, test_plot_environmental, test_plot_environmental_contour_multiyear
  • Status: Specified tests fail with error:
    'MATLAB:Python:PyException'
    --------------
    Error Details:
    --------------
    Error using Wave_TestResourceMetrics/test_plot_environmental_contour
    Python Error: TypeError: environmental_contours() missing 1 required positional argument: 'method'

Options:

  • Modify/Update:
    • Revise or update tests that call environmental_contour.
  • Deprecate:
    • Remove tests that call environmental_contour.

Additional Information

The specified tests call wave/resource/environmental_contour. The current version MHKiT-Python tests environmental contours here. In MHKIT-Python v0.6.0 this test was split from test_resource_metrics.py. To keep parity with MKHIT-Python, it may make sense to split these tests into a new file called Wave_TestContours and utilize a similar testing methodology as MHKiT-Python.

The root of this issue looks to be a the addition of the method argument to the environmental_contours function in MHKiT-Python, and changes to the structure of the returned dictionary.

The fix will require adding a new argument to environmental_contour and updating the code that converts the output of this function into a type that is useable in MATLAB.

@hivanov-nrel
Copy link
Contributor

The plot_environmental_contour_multi_year test in MATLAB should be removed as the python version of environmental_contours does not support the input of a list/vector as the return_period.

@hivanov-nrel hivanov-nrel linked a pull request Dec 30, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants