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.

Sign up for GitHub

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

Jump to bottom

Feature twotwo #367

Merged
merged 3 commits into from
Aug 6, 2020
Merged

Feature twotwo #367

merged 3 commits into from
Aug 6, 2020

Conversation

planes
Copy link
Member

@planes planes commented Aug 6, 2020

Pull Request Checklist

  • Merge in the latest Develop branch changes to your branch
  • Remove .pyc files from your repository:
    • Linux/Mac
      • find . -name *.pyc -delete
    • Windows
      • del /S *.pyc
  • Run automatic regression and makes sure everything is passing
  • Did you make a new regression test that covers your new code?
  • Did you update your docstrings?
  • Did you update your headers to include your name and date?
  • Do a final compare with the Develop branch to be sure what you're changing

@coveralls
Copy link

coveralls commented Aug 6, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.1%) to 86.837% when pulling 92f0b1d on feature_twotwo into 639bbc8 on develop.

@planes planes requested a review from mclarke2 August 6, 2020 02:13
mclarke2
mclarke2 reviewed Aug 6, 2020
View reviewed changes
trunk/SUAVE/Components/Energy/Converters/Propeller.py Outdated
@@ -524,6 +524,7 @@ def spin_variable_pitch(self,conditions):
ii = 0
broke = False
tol = 1e-6 # Convergence tolerance
print('Entering iteration')
Copy link
Member

Choose a reason for hiding this comment

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

I don't particularly like this print statement. 1) It is vague 2) we do not do this for other instances where we enter an iteration 3) this is an under the hood operation 4) the typical style of print statements is an output of an operation or a flag indicating ot the user that something is wrong. that being said on the BEMT branch, I added print statements to the two break conditions within this script

Copy link
Member Author

Choose a reason for hiding this comment

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

whoops, that wasn't supposed to be there. I put that there for a debug and thought I had deleted it

mclarke2
mclarke2 reviewed Aug 6, 2020
View reviewed changes
Copy link
Member

@mclarke2 mclarke2 left a comment

Choose a reason for hiding this comment

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

There is one issue within the Propeller.py script but besides that it looks good

mclarke2
mclarke2 approved these changes Aug 6, 2020
View reviewed changes
Copy link
Member

@mclarke2 mclarke2 left a comment

Choose a reason for hiding this comment

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

Nice!

@planes planes merged commit c7f69a3 into develop Aug 6, 2020
@planes planes deleted the feature_twotwo branch August 6, 2020 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mclarke2 mclarke2 mclarke2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@planes @coveralls @mclarke2