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

Options refactoring and docs update #38

Merged
merged 10 commits into from
Jan 19, 2021
Merged

Options refactoring and docs update #38

merged 10 commits into from
Jan 19, 2021

Conversation

sseraj
Copy link
Collaborator

@sseraj sseraj commented Jan 6, 2021

Purpose

This PR refactors the code to:

  1. Automatically generate the options documentation
  2. Make the USMesh class inherit from BaseSolver in baseclasses

The option type and value checking in BaseSolver breaks backward compatibility for some run scripts. This is usually only an issue for fileType because string-type values are now case sensitive. The actually functionality is unchanged.

The tutorial run scripts should be fine, but this will break the tests for:

  1. ADflow (I have fixed this in this PR)
  2. DAFoam (I will submit a PR after this is merged)
  3. pyAeroStructure (@eirikurj will fix the options either for the current tests or with the testflo update)

Once this is merged, the maintainers should make a new release and make these incompatibilities clear in the release notes. Here is an example.

One note for those more familiar with the code: PLOT3D mesh format looks like it is supported, but this is not mentioned anywhere in the options docs and is not tested. I have excluded plot3d from the fileType option for now. Let me know if I should add it.

Type of change

What types of change is it?
Select the appropriate type(s) that describe this PR

  • 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

See the BaseSolver tests for examples on how the type and value checking works.

Checklist

Put an x in the boxes that apply.

  • 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

@sseraj sseraj requested a review from a team as a code owner January 6, 2021 21:11
joanibal
joanibal previously approved these changes Jan 7, 2021
doc/options.yaml Show resolved Hide resolved
idwarp/UnstructuredMesh.py Outdated Show resolved Hide resolved
idwarp/UnstructuredMesh.py Outdated Show resolved Hide resolved
@sseraj
Copy link
Collaborator Author

sseraj commented Jan 17, 2021

After further discussion, we have decided to change the capitalization of the accepted fileType strings. This will break the current ADflow and DAFoam tests. I have updated the PR description accordingly. Tagging @friedenhe so you are aware.

Copy link
Collaborator

@ewu63 ewu63 left a comment

Choose a reason for hiding this comment

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

I think this one is ready right @eirikurj?

@ewu63 ewu63 requested a review from eirikurj January 19, 2021 21:55
@eirikurj eirikurj merged commit 485f8c3 into mdolab:master Jan 19, 2021
@sseraj sseraj deleted the options branch January 19, 2021 22:19
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.

4 participants