export to STL format capability added - #176
Conversation
gonuke
left a comment
gonuke
left a comment
There was a problem hiding this comment.
Thanks for this useful addition @davidsosa. I've recommended that we implement it as a single function that exports either STEP or STL since they are identical functions. There may be an argument to keep both export_step and export_stl, but have them each call the same function that performs the work with an optional file type.
| ) | ||
| cq.exporters.export(component, str(export_path)) | ||
|
|
||
| def export_stl(self, export_dir=""): |
There was a problem hiding this comment.
Since this function is functionally identical to export_step it might be better to rename export_step as export_standard (or something) and add the export type as an argument.
There was a problem hiding this comment.
You could change export_step to export_cad and have the function take the file extension as an argument (in string format)
| ) | ||
| cq.exporters.export(coil_set, str(export_path)) | ||
|
|
||
| def export_stl(self, stl_filename="magnet_set", export_dir=""): |
There was a problem hiding this comment.
Similar comment as above about extending the previous method rather than copy/pasting to a new method.
There was a problem hiding this comment.
I see. I did this way because It was usefull for me to get these two format at the same time, but for sure it can be done extending the method instead of duplicating it. I'l have a deeper look on it
There was a problem hiding this comment.
You could still get both in the same run, by just calling the method twice with different arguments.
666c81c to
b1e9317
Compare
gonuke
left a comment
gonuke
left a comment
There was a problem hiding this comment.
This looks good to me. Do we currently test this for STEP? Should we add a test for STL?
connoramoreno
left a comment
connoramoreno
left a comment
There was a problem hiding this comment.
Just some general comments and thoughts on naming and inputs that could possibly use some discussion. We'll also need to update the test suite to reflect the modified API.
| def export_components(self, filetype="step", export_dir=""): | ||
| """Export CAD solids as STEP or STL files via CadQuery. |
There was a problem hiding this comment.
It might be a good idea to include some kind of check on filetype input, unless we want to rely on CadQuery to issue an error if an incorrect filetype is supplied.
| def export_components( | ||
| self, filetype="step", filename="magnet_set", export_dir="" | ||
| ): |
There was a problem hiding this comment.
Would it make sense to combine filename and filetype into a single input argument? This would deviate from our established convention.
|
|
||
| def export_step(self, step_filename="magnet_set", export_dir=""): | ||
| """Export CAD solids as a STEP file via CadQuery. | ||
| def export_components( |
There was a problem hiding this comment.
Instead of calling this export_components, since there's only one component being exported, maybe we should call this something like export_cad, and do the same with the in-vessel build for consistency.
3 participants
I added the option to export geometry files in STL format for both magnets and in-vessel components.