-
Notifications
You must be signed in to change notification settings - Fork 30
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
Inclusion of Shaft and Propeller classes with tests (basic version) #62
Conversation
bladex/propeller.py
Outdated
sewed_full_body = sewer.SewedShape() | ||
self.sewed_full_body = sewer.SewedShape() | ||
|
||
def generate_propeller_iges(self, propeller_and_shaft, display=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generate_iges
for the method name (no reason to be redundant with "propeller" since the object itself is called Propeller
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, thank you.
bladex/propeller.py
Outdated
display, start_display, add_menu, add_function_to_menu = init_display() | ||
display.DisplayShape(self.sewed_full_body, update=True) | ||
start_display() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why display when exporting? It looks much better to me have a separate method to display the propeller like>
my_custom_propeller = Propeller(...)
my_custom_propeller.display()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change accordingly, thanks.
bladex/propeller.py
Outdated
start_display() | ||
|
||
def generate_propeller_stl(self, propeller_and_shaft, display=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, thanks.
bladex/propeller.py
Outdated
display, start_display, add_menu, add_function_to_menu = init_display() | ||
display.DisplayShape(self.sewed_full_body, update=True) | ||
start_display() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change accordingly, thanks.
bladex/shaft.py
Outdated
self.shaft_path = shaft_path | ||
self.display = display | ||
|
||
def generate_shaft_solid(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generate_solid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks.
bladex/shaft.py
Outdated
if self.display: | ||
display, start_display, add_menu, add_function_to_menu = init_display() | ||
display.DisplayShape(shaft_solid, update=True) | ||
start_display() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change accordingly, thanks.
bladex/propeller.py
Outdated
|
||
:param shaft.Shaft shaft: shaft to be added to the propeller | ||
:param blade.Blade blade: blade of the propeller | ||
:param int n_blades: number of blades componing the propeller | ||
:cvar OCC.Core.TopoDS.TopoDS_Solid shaft_solid: solid shaft | ||
:cvar OCC.Core.TopoDS.TopoDS_Shell: propeller with shaft shell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing name of the variable
bladex/propeller.py
Outdated
|
||
:param string filename: path (with the file extension) where to store | ||
the .stl CAD for the propeller and shaft |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a tab is missing. Please check the doc is properly generated
@@ -8,27 +8,25 @@ class Shaft(object): | |||
""" | |||
Bottom-up parametrized shaft construction. | |||
|
|||
:param string shaft_path: path of a .iges file with stored shaft information. | |||
:param bool display: if True, then display the shaft. Default value is False. | |||
:cvar string filename: path (with the file extension) of a .iges file with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a param, not only a var
The inclusion of Shaft and Propeller classes has to be viewed as a basic first version of them, based on already developed tools. Actually, the solid shaft is built reading from a file with stored shaft information. The propeller (including shaft) is obtained building on the Shaft and the already existing Blade class. I hope that this predisposition of codes could be useful as a starting point for further and future developments.