346 TimeSeries interp_time

[vhirtham]

Changes

TimeSeries.interp_time now returns a new TimeSeries instead of an xarray.DataArray. Additionally, a warning is emitted when already interpolated data is interpolated another time.

Related Issues

Closes #346

Checks

• updated CHANGELOG.md
• updated tests
• updated doc/
• update example/tutorial notebooks
• save interpolation counter to asdf?

[vhirtham]

The CI failures are mostly things that are already adjusted in the measurement chain PR. So I suggest we wait until this one is merged and fix the remaining errors then.

Also, I added an internal interpolation counter to emit a warning for interpolating the same data over and over again. Do we want to store the counter in a Weldx file too? Guess it would make sense.

[CagtayFabry]

what do you think about adding a .time property to TimeSeries directly @vhirtham ? In line with LCS and similar classes

[vhirtham]

what do you think about adding a .time property to TimeSeries directly @vhirtham ? In line with LCS and similar classes

Not sure what you intend, cause the TimeSeries already has a time property (line 501)

[CagtayFabry]

what do you think about adding a .time property to TimeSeries directly @vhirtham ? In line with LCS and similar classes

Not sure what you intend, cause the TimeSeries already has a time property (line 501)

exactly that, so disregard 😉
just wondering why we now need to call data_array.time here https://github.com/vhirtham/weldx/blob/3b06b74a800d2ac45b9794dfd81d49670d63f4e9/weldx/tests/test_core.py#L416-L419

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov]

Codecov Report

Merging #353 (131d414) into master (ca354e1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #353   +/-   ##
=======================================
  Coverage   97.43%   97.44%           
=======================================
  Files          86       86           
  Lines        5176     5196   +20     
=======================================
+ Hits         5043     5063   +20     
  Misses        133      133           

Impacted Files	Coverage Δ
weldx/core.py	99.47% <100.00%> (+0.06%)	⬆️
weldx/util.py	98.75% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca354e1...131d414. Read the comment docs.

[vhirtham]

what do you think about adding a .time property to TimeSeries directly @vhirtham ? In line with LCS and similar classes

Not sure what you intend, cause the TimeSeries already has a time property (line 501)

exactly that, so disregard 😉
just wondering why we now need to call data_array.time here https://github.com/vhirtham/weldx/blob/3b06b74a800d2ac45b9794dfd81d49670d63f4e9/weldx/tests/test_core.py#L416-L419

The reason was that .time returns None if the internal time data has only one value, e.g the TS is constant. But we checked is the stored value matches the passed one. Don't know if that made sense, but I modified the test in favor of using .time

[CagtayFabry]

LGTM 👍