mdolab / pygeo

pyGeo provides geometric design variables and constraints suitable for gradient-based optimization.
https://mdolab-pygeo.readthedocs-hosted.com/en/latest/?badge=latest
Apache License 2.0
122 stars 54 forks source link

Moved OpenVSP version check to fix API docs #210

Closed sseraj closed 1 year ago

sseraj commented 1 year ago

Purpose

198 broke the API docs for DVGeometryVSP, the modules that follow it in __init__.py (DVGeometryESP, DVGeometryMulti), and the MPhys wrapper. The problem is that the openvsp.GetVSPVersion() call occurs outside the class, which does not work when we have mock imports for the doc build. To fix this, I moved the version check into the __init__ function.

Expected time until merged

1 week

Type of change

Testing

Check the API docs for the latest docs build and compare to the PR build. All the API docs should be working on the PR build.

Checklist

codecov[bot] commented 1 year ago

Codecov Report

Merging #210 (12482d0) into main (dc41b1f) will not change coverage. The diff coverage is 91.66%.

@@           Coverage Diff           @@
##             main     #210   +/-   ##
=======================================
  Coverage   64.76%   64.76%           
=======================================
  Files          47       47           
  Lines       12018    12018           
=======================================
  Hits         7784     7784           
  Misses       4234     4234           
Impacted Files Coverage Δ
pygeo/parameterization/DVGeoESP.py 66.57% <ø> (ø)
pygeo/parameterization/DVGeoVSP.py 81.97% <91.66%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

sseraj commented 1 year ago

@sseraj there is a unit test for the out of date features, but can you expand it to check the different version conditions? I think the machinery is there to do it fairly easily.

I don't think there is a good way of doing this with a dummy OpenVSP module. The other version conditions don't raise an error, so the entire __init__ function runs when I try to test it. To properly test multiple versions of OpenVSP, we would have to set different stable and latest versions in our Docker images.

lamkina commented 1 year ago

Okay, I guess it's fine. We're throwing a descriptive error message about the version so the user will be made aware of the problem.