mdolab / openconcept

OpenConcept: A toolkit for conceptual MDAO of aircraft with unconventional propulsion architectures
MIT License
36 stars 32 forks source link

Transition to Github Actions #26

Closed bernardopacini closed 3 years ago

bernardopacini commented 3 years ago

Purpose

This change adds Github Actions and removes Travis testing. The Github Actions configuration file is in .github/workflows/. Also, this change pins the OpenMDAO version in requirements.txt to match the one tested in travis, the latest version fails the tests.

Type of change

This is a maintenance update.

Select the appropriate type(s) that describe this PR

Testing

Tests pass on personal fork, push to coveralls not tested yet (must come from MDOLab repo).

Checklist

Put an x in the boxes that apply.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 570172305


Files with Coverage Reduction New Missed Lines %
openconcept/utilities/visualization.py 2 15.38%
<!-- Total: 2 -->
Totals Coverage Status
Change from base Build 178: -0.05%
Covered Lines: 3728
Relevant Lines: 4261

💛 - Coveralls
bernardopacini commented 3 years ago

Addresses issue #24.

bernardopacini commented 3 years ago

Testing with Github Actions is up and running besides the push to Coveralls. @bbrelje Do you remember how you set that up originally? Do we need to reset the repo on Coveralls for it to recognize Github Actions instead of Travis?

EDIT: Fixed (see below)

bernardopacini commented 3 years ago

Fixed coveralls push by switching to coveralls package instead of python-coveralls. The latter appears to be geared towards Travis specifically. This switch is also consistent with OpenAeroStruct which uses coveralls.

bernardopacini commented 3 years ago

Before merging, Travis should be removed and Github Actions should be set as the required check. I do not believe I have access to make this change.

Also, AppVeyor is failing because it uses the wrong version of OpenMDAO (3.7 as opposed to 3.2.1). Do we want to fix this and continue using AppVeyor or remove it to rely on Github Actions?

bernardopacini commented 3 years ago

Fixed AppVeyor by pinning OpenMDAO version (3.2.1). Github Actions can test on Windows. Do we want to continue running AppVeyor or consolidate to Github Actions with two stages, one for linux and one for windows?

ewu63 commented 3 years ago

I would prefer to pin the version in setup.py rather than in requirements.txt. Also, not sure where environment.yml is used.

I'm also ambivalent regarding AppVeyor, though if we can do exactly the same in GA I guess that would be nice.

EDIT: I see that environment.yml is used for conda. I would prefer to unify everything into setup.py, and then we should be able to install pip from conda and use that to install all dependencies on Windows I think.

bernardopacini commented 3 years ago

I pinned the version of OpenMDAO in setup.py, but left it pinned in requirements.txt as that is executed before setup.py is referenced. If we want to consolidate to setup.py and have that install all the dependencies when it is called, I can remove requirements.txt completely.

I removed the AppVeyor config file and environments.yml. The windows build / test is now handled by Actions.

ewu63 commented 3 years ago

I pinned the version of OpenMDAO in setup.py, but left it pinned in requirements.txt as that is executed before setup.py is referenced. If we want to consolidate to setup.py and have that install all the dependencies when it is called, I can remove requirements.txt completely.

Yep I would prefer letting pip handle everything via setup.py rather than having a separate file that must be pip-installed first.

I removed the AppVeyor config file and environments.yml. The windows build / test is now handled by Actions.

Any idea why AppVeyor still ran? Maybe we have to manually remove the project from their side? Might need Ben's help on that.

bernardopacini commented 3 years ago

I’ll condense it and remove the requirements file.

I am not sure about AppVeyor, I am not familiar with how it works. I imagine it has a hook that triggers a build there no matter what, so we’ll need to remove that.

bernardopacini commented 3 years ago

@bbrelje I am thinking of moving all of the required packages (besides coverage-related ones) into the setup.py file so they are installed automatically. This would be: sphinx, sphinx_rtd_theme, and redbaron. Do you have any issue with this?

ewu63 commented 3 years ago

Any idea where the redbaron dependency comes from? I feel like I've seen that before

kanekosh commented 3 years ago

Any idea where the redbaron dependency comes from? I feel like I've seen that before

I think redbaron is one of the OpenMDAO requirements to build the sphinx docs.

ewu63 commented 3 years ago

I think redbaron is one of the OpenMDAO requirements to build the sphinx docs.

Right thank you! We probably want to do openmdao[docs] or something then. I will try that

ewu63 commented 3 years ago

Sorry for the excessive commits, after a really long time I figured out that for miniconda to work we had to set bash differently. Everything is in place now, and I also incremented the version.

ewu63 commented 3 years ago

@bbrelje @kanekosh this PR is ready for you to review.