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

Add units to ResultsIndex #330

Merged
merged 5 commits into from
Mar 13, 2023
Merged

Add units to ResultsIndex #330

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e73339
Expose data unit for ResultIndex
PProfizi Mar 10, 2023
6a142ff
Update test
PProfizi Mar 10, 2023
f0e8311
Improve coverage
PProfizi Mar 10, 2023
df7245e
Improve readability
PProfizi Mar 13, 2023
3a90a63
Merge branch 'master' into feat/results_index_unit
PProfizi Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/ansys/dpf/post/index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,24 @@ class ResultsIndex(Index):
def __init__(
self,
values: List[str],
units: Union[List[Union[str, None]], None] = None,
):
"""Initiate this class."""
super().__init__(name=ref_labels.results, values=values, scoping=None)
result_values = values
if units is not None:
for i, unit in enumerate(units):
result_values[i] += f" ({unit})" if unit is not None else ""
if len(units) < len(values):
units.extend([None] * (len(values) - len(units)))
else:
units = [None] * len(values)
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably we want to avoid writing "()" if unit is None. :)

And personally, I read better the if before the for. But this is a nit pick :).

Suggested change
for i, unit in enumerate(units):
result_values[i] += f" ({unit})" if unit is not None else ""
if len(units) < len(values):
units.extend([None] * (len(values) - len(units)))
else:
units = [None] * len(values)
if len(units) < len(values):
units.extend([None] * (len(values) - len(units)))
for i, unit in enumerate(units):
result_values[i] = f" ({unit})" if unit is not None
else:
units = [None] * len(values)

Copy link
Contributor Author

@PProfizi PProfizi Mar 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Probably we want to avoid writing "()" if unit is None. :)

And personally, I read better the if before the for. But this is a nit pick :).

Hi @germa89! Thx for the review! Just to nit pick too, the original line 174 does actually write "" if unit is None ;)

Also, the if after the for is because the operation of adding the unit to the string rep is not needed for None values of units, which is all we are adding in the if.
Thus having the if before the for might be a bit more readable, but is adds unnecessary operations. What I will do is add comments so it is more readable because you are right, it is not really readable.

self._units = units
super().__init__(name=ref_labels.results, values=result_values, scoping=None)

@property
def units(self) -> List[str]:
"""Return the list of units for this index."""
return self._units

def __repr__(self):
"""Representation of the Index."""
Expand Down
3 changes: 2 additions & 1 deletion src/ansys/dpf/post/simulation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,14 +523,15 @@ def _create_dataframe(self, fc, location, columns, comp, base_name, disp_wf=None
message=f"Returned Dataframe with columns {columns} is empty.",
category=UserWarning,
)
unit = fc[0].unit
comp_index = None
if comp is not None:
comp_index = CompIndex(values=comp)
row_indexes = [MeshIndex(location=location, fc=fc)]
if comp_index is not None:
row_indexes.append(comp_index)
column_indexes = [
ResultsIndex(values=[base_name]),
ResultsIndex(values=[base_name], units=[unit]),
SetIndex(values=fc.get_available_ids_for_label("time")),
]
label_indexes = []
Expand Down
20 changes: 13 additions & 7 deletions tests/test_dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,15 @@ def test_dataframe_animate(transient_rst):

def test_dataframe_repr(df):
ref = (
"DataFrame<index=MultiIndex<[MeshIndex<name=\"node_ids\", dtype=<class 'int'>>, "
"Index<name=\"components\", dtype=<class 'str'>>]>, columns=MultiIndex<[ResultIndex<['U']>, " # noqa: E501
"SetIndex<values=[1]>]>>"
"DataFrame<"
"index=MultiIndex<["
"MeshIndex<name=\"node_ids\", dtype=<class 'int'>>, "
"Index<name=\"components\", dtype=<class 'str'>>"
"]>, "
"columns=MultiIndex<["
"ResultIndex<['U (m)']>, "
"SetIndex<values=[1]>"
"]>>"
)
assert repr(df) == ref
Comment on lines 141 to 152
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to use this pytest plugin to automatize "golden" tests

https://github.com/oprypin/pytest-golden#pytest-golden

Feel free to ping me about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh nice, thx @germa89, I did not know this plugin!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know if it works well for numerical values?


Expand All @@ -151,7 +157,7 @@ def test_dataframe_str(transient_rst):
df = simulation.displacement(all_sets=True)
print(df)
ref = """
results U ...
results U (m) ...
set_ids 1 2 3 4 5 6 ...
node_ids components ...
525 X 0.0000e+00 4.8506e-05 2.3022e-04 6.5140e-04 1.4812e-03 2.9324e-03 ...
Expand All @@ -166,7 +172,7 @@ def test_dataframe_str(transient_rst):
df2 = df.select(node_ids=525)
print(df2)
ref = """
results U ...
results U (m) ...
set_ids 1 2 3 4 5 6 ...
node_ids components ...
525 X 0.0000e+00 4.8506e-05 2.3022e-04 6.5140e-04 1.4812e-03 2.9324e-03 ...
Expand All @@ -179,7 +185,7 @@ def test_dataframe_str(transient_rst):
print(df)
print(df._fc[0].get_entity_data_by_id(391))
ref = """
results S
results S (Pa)
set_ids 35
element_ids components
391 XX (1) -3.2780e+05
Expand All @@ -194,7 +200,7 @@ def test_dataframe_str(transient_rst):
df2 = df.select(components="YY")
print(df2)
ref = """
results S
results S (Pa)
set_ids 35
element_ids components
391 YY (1) 1.3601e+06
Expand Down
27 changes: 27 additions & 0 deletions tests/test_index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from ansys.dpf.post.index import ResultsIndex


class TestResultsIndex:
def test_results_index(self):
result_index = ResultsIndex(values=["U", "V", "A"], units=None)
ref = "ResultIndex<['U', 'V', 'A']>"
assert repr(result_index) == ref
result_index = ResultsIndex(values=["U", "V", "A"], units=["m", "m/s"])
ref = "ResultIndex<['U (m)', 'V (m/s)', 'A']>"
assert repr(result_index) == ref
result_index = ResultsIndex(values=["U", "V", "A"], units=["m", None, "m/s^2"])
ref = "ResultIndex<['U (m)', 'V', 'A (m/s^2)']>"
assert repr(result_index) == ref
result_index = ResultsIndex(values=["U", "V", "A"], units=["m", "m/s", "m/s^2"])
ref = "ResultIndex<['U (m)', 'V (m/s)', 'A (m/s^2)']>"
assert repr(result_index) == ref

def test_results_index_units(self):
result_index = ResultsIndex(values=["U", "V", "A"], units=None)
assert result_index.units == [None, None, None]
result_index = ResultsIndex(values=["U", "V", "A"], units=["m", "m/s"])
assert result_index.units == ["m", "m/s", None]
result_index = ResultsIndex(values=["U", "V", "A"], units=["m", None, "m/s^2"])
assert result_index.units == ["m", None, "m/s^2"]
result_index = ResultsIndex(values=["U", "V", "A"], units=["m", "m/s", "m/s^2"])
assert result_index.units == ["m", "m/s", "m/s^2"]