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

Docs update #389

Merged
merged 18 commits into from
Jul 15, 2022
Merged

Docs update #389

merged 18 commits into from
Jul 15, 2022

Conversation

lamkina
Copy link
Contributor

@lamkina lamkina commented Jun 11, 2022

Purpose

This PR uses the sphinx extensions added to sphinx_mdolab_theme that were originally from OpenMDAO v3.9.2. The purpose is to use these extensions to have code blocks run dynamically when we build the docs and generate necessary outputs automatically. This replaces the current static documentation format.

Expected time until merged

This is a non-urgent PR and might take some time to iron out all the bugs and clean up the docs and comments in the testing code.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

The only testing for this PR is building the docs and making sure they look correct with the new extensions.

Checklist

  • I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@codecov
Copy link

codecov bot commented Jun 11, 2022

Codecov Report

Merging #389 (0a4ee0e) into main (614dc6a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #389   +/-   ##
=======================================
  Coverage   96.63%   96.63%           
=======================================
  Files          93       93           
  Lines        6009     6009           
=======================================
  Hits         5807     5807           
  Misses        202      202           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@lamkina lamkina marked this pull request as ready for review July 12, 2022 15:47
@lamkina lamkina requested a review from a team as a code owner July 12, 2022 15:47
Copy link
Collaborator

@eytanadler eytanadler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is a great improvement and really cleans up the code, thanks Andrew!

One question: do we have control over the output font size? It looks substantially bigger than the code or rest of the docs.

openaerostruct/docs/advanced_features/aerostruct_ffd.rst Outdated Show resolved Hide resolved
print(prob["aero_point_1.wing_perf.CL"][0])
print(prob["aero_point_1.wing_perf.CD"][0])
.. embed-code::
openaerostruct.tests.test_multipoint_aero.Test.test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also doesn't look like anything is printed out here? Is that the intended behavior if there is no :layout: specified?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can specify a layout for these if we want the output. I think I added the output manually when I went made the initial docs update. The older version of the docs didn't include the output.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I don't think it'd hurt given how easy it is. Anyone else have thoughts? @kanekosh?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do it for now. I think the output makes it easier for people to compare their scripts when they are learning the examples.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it’s better to print the output values here.

openaerostruct/docs/user_reference/v1_v2_conversion.rst Outdated Show resolved Hide resolved
@eytanadler
Copy link
Collaborator

I agree that it's better to have the output there in the docs. I didn't comment in every place that it was missed, do you mind adding in a :layout: everywhere it's relevant? For example the multipoint optimization doesn't show the output.

@lamkina lamkina requested a review from eytanadler July 13, 2022 17:04
@eytanadler
Copy link
Collaborator

@kanekosh do you mind taking a quick look at this before we merge?

Copy link
Contributor

@kanekosh kanekosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @lamkina!

@kanekosh kanekosh merged commit 38f07ea into mdolab:main Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants