search

MHKiT-Software / MHKiT-MATLAB

MHKiT-MATLAB provides the marine renewable energy (MRE) community tools for data processing, visualization, quality control, resource assessment, and device performance.
https://mhkit-software.github.io/MHKiT/
BSD 3-Clause "New" or "Revised" License
15 stars 23 forks source link

Move CI Unit Tests to GitHub Actions #80

Closed H0R5E closed 2 years ago

H0R5E commented 2 years ago

This PR moves the continuous integration unit tests to GitHub Actions.

There are a number of changes and notes, as follows:

Matthew-Boyd commented 2 years ago

@rpauly18 Everything looks good to me, but there's changes to a few different files so I won't accept the pull request myself.

H0R5E commented 2 years ago

This is real great Mat, thanks for figuring this out for us. I see the couple key things you solved, and thanks also for everything else. Filling in the remaining blanks in the workflow and fixing the causes of the slow tests--that's a nice touch that's much appreciated.

I'm glad to see they've added macOS and Windows support! I'll keep an eye on your submitted Windows Issue; hopefully they get that resolved.

@Matthew-Boyd, no problem, glad I could be of help. I've spent enough time banging my head against the wall with these MATLAB CI runners to know their foibles by now, but it still took me longer to do than I anticipated!

I'm hoping someone will be able to address the Windows issue soon. Generally, it doesn't take too long for MathWorks to respond - although a fix can be further down the line.

Just a reminder that I had to skip the tests for the environmental_contour function. I wasn't sure if you guys were aware it is broken or not.

Matthew-Boyd commented 2 years ago

@Matthew-Boyd, no problem, glad I could be of help. I've spent enough time banging my head against the wall with these MATLAB CI runners to know their foibles by now, but it still took me longer to do than I anticipated!

I'm hoping someone will be able to address the Windows issue soon. Generally, it doesn't take too long for MathWorks to respond - although a fix can be further down the line.

Just a reminder that I had to skip the tests for the environmental_contour function. I wasn't sure if you guys were aware it is broken or not.

You saved me some additional head trauma, so thanks!

Understood about the environmental_contour function--we'll look into it.

tacaswell commented 2 years ago

@H0R5E Did you tag the right Matplotlib issue? The code implicated by https://github.com/matplotlib/matplotlib/issues/22378 goes back to mpl 2.1

H0R5E commented 2 years ago

@tacaswell, thanks for your comment. I think you are right, in that pinning the version of matplotlib <3.4.2 is totally coincidental. The bug I am getting is actually really hard to recreate. I have one MATLAB session open outside of my Python environment, with an editor and a plot window open, and then a MATLAB session open with an editor window open. Then when I run the MHKIT-MATLAB tests I get this:

   ---------
    Error ID:
    ---------
    'MATLAB:Python:PyException'
    --------------
    Error Details:
    --------------
    Error using _backend_tk><module> (line 24)
    Python Error: RuntimeError: Failed to load Tcl_SetVar

    Error in backend_tkagg><module> (line 1)

    Error in __init__>import_module (line 127)

    Error in pyplot>backend_mod (line 268)

    Error in pyplot>switch_backend (line 267)

    Error in pyplot>switch_backend (line 247)

    Error in __init__>__getitem__ (line 672)

    Error in pyplot><module> (line 2230)

    Error in graphics><module> (line 4)

    Error in __init__><module> (line 3)

    Error in __init__><module> (line 1)

    Error in <frozen importlib>_call_with_frames_removed (line 219)

    Error in <frozen importlib>exec_module (line 728)

    Error in <frozen importlib>_load_unlocked (line 677)

    Error in <frozen importlib>_find_and_load_unlocked (line 967)

    Error in <frozen importlib>_find_and_load (line 983)

    Error in <frozen importlib>_gcd_import (line 1006)

    Error in __init__>import_module (line 127)

My assumption is that this is due to https://github.com/matplotlib/matplotlib/issues/22378, but I'm guessing pretty convinced that my remedy is bogus, as I probably just changed the conditions that I was running the tests in.

@Matthew-Boyd, @rpauly18, @ssolson I am going to put in a PR to delete the pinning of matplotlib in the workflow, as this seems like a edgy case in Windows. that we are not testing now anyway. <-- EDIT: Oh yes we are. 🥇

H0R5E commented 2 years ago

Please see #81.

tacaswell commented 2 years ago

Can you test if https://github.com/matplotlib/matplotlib/pull/22445 fixes the problem for you?

tacaswell commented 2 years ago

And in the contexts of your use of Matplotlib inside of MATLAB code, do you actually want to be be using a GUI toolkit?

H0R5E commented 2 years ago

Can you test if matplotlib/matplotlib#22445 fixes the problem for you?

@tacaswell, I'm happy to give it a go. I assume I need to build matplotlib with your branch to test it?

And in the contexts of your use of Matplotlib inside of MATLAB code, do you actually want to be using a GUI toolkit?

It's a good point, but I'm not responsible for the design of this project. As far as I understand, the upstream Python package takes priority.