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

Create test_cpdAssignUStarTh20100901.m #28

Open
wants to merge 13 commits into
base: 37-modularise-cpdBoostrap
Choose a base branch
from

Conversation

j-emberton
Copy link
Member

@j-emberton j-emberton commented Sep 10, 2024

Closes #27

Added unit tests for cpdAssignUStarTh20100901 and updated conftest.py with a MFWrapper to wrap the matlab function. With MFWrapper, any function call by matlab_engine.FUNC will automatically handle the JSON encoding and decoding of non-scalar structs, as long as in the Python side we add keyword arguments jsonencode=[...] or jsondecode=[...] to matlab_engine.FUNC(). The wrapper also will show the printing in MATLAB functions at the end of the testing.

@tztsai tztsai marked this pull request as ready for review October 21, 2024 10:38
@tztsai tztsai changed the title Create test_cpdAssignUStarTh20100901.m [WIP] Create test_cpdAssignUStarTh20100901.m Oct 21, 2024
@dorchard
Copy link
Member

Quite a few of the tests fail for me. Some seem to be to do with the creating of a struct to pass to MATLAB:

FAILED tests/unit_tests/test_ustar_cp/test_cpdAssignUStarTh20100901.py::test_cpdAssignUStarTh20100901_basic - AttributeError: 'dict' object has no attribute 'mt'
FAILED tests/unit_tests/test_ustar_cp/test_cpdAssignUStarTh20100901.py::test_cpdAssignUStarTh20100901_plotting - AttributeError: 'dict' object has no attribute 'mt'

Others seem to be to do with the matlab engine setup.

I can't see why this would be a local problem for me in the latter case.

@tztsai tztsai changed the base branch from ustar_cp_refactor_main to 29-generate-unit-tests-for-cpdbootstrap October 24, 2024 13:16
@tztsai tztsai force-pushed the 27-generate-unit-tests-for-cpdassignustar branch from 3532819 to 3066924 Compare October 24, 2024 13:17
@dorchard dorchard marked this pull request as draft October 24, 2024 14:54
@tztsai tztsai marked this pull request as ready for review October 28, 2024 17:43
@tztsai tztsai requested a review from dorchard October 28, 2024 17:43
@tztsai tztsai changed the base branch from 29-generate-unit-tests-for-cpdbootstrap to 37-modularise-cpdBoostrap October 28, 2024 19:11
@tztsai tztsai force-pushed the 27-generate-unit-tests-for-cpdassignustar branch from e3983df to 7b0b7f7 Compare October 28, 2024 19:16
@tztsai tztsai self-requested a review November 4, 2024 09:19
@dorchard
Copy link
Member

dorchard commented Nov 7, 2024

Thanks @tztsai - I'm having a problem with the tests here tests/unit_tests/test_ustar_cp/test_cpdBootstrapUStarTh4Season20100901.py::test_cpdBootstrap_against_testcases seems to be hanging but I can't see why. I think it must be a local problem (two tests run then things get stuck).

@dorchard
Copy link
Member

dorchard commented Nov 7, 2024

I think I've narrowed it down to tests/unit_tests/test_ustar_cp/test_cpdBootstrapUStarTh4Season20100901.py on line 143... which is strange.

@tztsai
Copy link

tztsai commented Nov 7, 2024

Now the handling of NaN and None values and their comparison are fixed. All tests pass.

@dorchard dorchard linked an issue Nov 7, 2024 that may be closed by this pull request
@dorchard
Copy link
Member

dorchard commented Nov 8, 2024

@tztsai Thanks! looks good to me. Could you resolve the conflicts with the target branch then I am happy to approve for merge.

@tztsai
Copy link

tztsai commented Nov 11, 2024

@tztsai Thanks! looks good to me. Could you resolve the conflicts with the target branch then I am happy to approve for merge.

Resolved now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None vs NaN
3 participants