Cambridge-ICCS / ONEFlux

Open Network-Enabled Flux processing pipeline
Other
0 stars 0 forks source link

Create test_cpdAssignUStarTh20100901.m #28

Closed j-emberton closed 1 week ago

j-emberton commented 2 months ago

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.

dorchard commented 4 weeks ago

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.

dorchard commented 2 weeks ago

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 commented 2 weeks ago

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

tztsai commented 2 weeks ago

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

dorchard commented 2 weeks ago

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

tztsai commented 1 week ago

@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