ExcelConceptEV #500
ExcelConceptEV #500
Conversation
ravi-ansys
requested review from
jgsdavies,
JackB-Ansys,
matthewAnsys and
james-packer
as code owners
ravi-ansys
added
enhancement
| @@ -32,6 +32,10 @@ | |||
| k_custom_loss_power_function_external_lab = "CustomLoss_PowerFunction_External_Lab" | |||
| k_custom_loss_voltage_function_external_lab = "CustomLoss_VoltageFunction_External_Lab" | |||
|
|
|||
| import numpy as np | |||
There was a problem hiding this comment.
What will happen if numpy and scipy aren't available? They aren't in the main dependencies (only in the test dependencies). I think in other cases we detect and warn if some specific dependencies aren't available. At least until now I think we have been trying to avoid adding large things like numpy/scipy to the required dependencies.
There was a problem hiding this comment.
Hi James,
May I confirm,
I have looked into the installed folder of python with Motor-CAD. It seems numpy and scipy is already available.
There was a problem hiding this comment.
@ravi-ansys, scipy (and therefore numpy) are included in the Python environment distributed with the Motor-CAD installation, as you show above. However, they are only optional (documentation) dependencies of PyMotorCAD itself (see https://github.com/ansys/pymotorcad/blob/main/pyproject.toml).
There are two options, one is to move scipy into the main dependencies (which we have avoided until now), the other is to check when you import it if it's available, and fail with a useful error message if it isn't available. For an example of this, have a look at how MATPLOTLIB_AVAILABLE is used in https://github.com/ansys/pymotorcad/blob/main/src/ansys/motorcad/core/geometry_drawing.py
@jgsdavies, any opinion on this?
There was a problem hiding this comment.
I prefer the MATPLOTLIB_AVAILABLE method for now. We can also add an optional dependency list in the same way as we do docs and tests in pyproject.yml. Maybe CEV can also have this?
| ii = 0 | ||
| jj = 0 | ||
| if sheet == "Speed": | ||
| for col in ws.iter_cols(min_col=0, max_col=j_len, max_row=i_len): |
There was a problem hiding this comment.
use "enumerate" for the index variables ii and jj
| @@ -304,3 +308,166 @@ def export_lab_model(self, file_path): | |||
| method = "ExportLabModel" | |||
| params = [file_path] | |||
| return self.connection.send_and_receive(method, params) | |||
|
|
|||
| @staticmethod | |||
There was a problem hiding this comment.
lets just make this a function rather than a staticmethod
There was a problem hiding this comment.
I think if I make this function than it will show up in documentation which we may not want .This is supporting function to the main function export_concept_ev_model. @jgsdavies can confirm .
There was a problem hiding this comment.
I don't think it's going to add functions with underscores
|
It's odd that the thermal tests are failing (test_get_node_to_node_resistance, test_get_node_power), I can't see how these are related to the code changes. @ravi-ansys, do you get these failures if you run the tests locally? |
|
|
@philipjusher Hi Phill, I have update the branch with the reviews. It seems to pass test also this time. Let me know if any further issues need addressing. |
Added a function to export Efficient map data of LAB as excel file suitable for ConceptEV