SpatialSeries and DynamicTraceSegment
#699
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
This looks really cool 👍 🚀 ! (and super fun to play with 😎 ) I know it's not final but I'll add some quick comments as notes in the code |
CHANGELOG.rst
Outdated
| @@ -9,7 +9,7 @@ | |||
| added | |||
| ===== | |||
|
|
|||
| - `SpatialSeries` and `DynamicTraceSegment` [:pull:`699`] | |||
| - ``SpatialSeries`` and ``DynamicTraceSegment`` [:pull:`699`] | |||
There was a problem hiding this comment.
Seems like sphink does not find those two. I already added them to the __init__.py. Is there another place I forgot?
There was a problem hiding this comment.
have you tried ~weldx.core.SpatialSeries ?
Maybe it works if you add it to the user API documentation in weldx/__init__.py
There was a problem hiding this comment.
I used the previous method already in the docstrings and it worked. Was just wondering why it is necessary for some of our classes while others don't need it. Your second suggestion at least worked for the changelog.
There was a problem hiding this comment.
I think because of the user API some elements (our main classes) appear two times in the sphinx tree, e.g. once as weldx.time.Time and once as weldx.Time (user API)
In these cases the references have to be explicit or sphinx might throw an error (because it doesn't know which page to link to). But that should be mostly on the API documentation site
For the changelog I am nur sure what the sphinx context is for resolving these (or if there is any difference)
weldx/geometry.py
Outdated
|
|
||
| self._length = self.get_section_length(self._max_s) | ||
|
|
||
| def _get_component_derivative_squared(self, i: int): |
There was a problem hiding this comment.
can you add the return value for the type hint here @vhirtham
Is it a sympy expression?
There was a problem hiding this comment.
I will try it. When I printed the type, I got a derived type. I have to figure out the base type
There was a problem hiding this comment.
Just some minor details, I have also modified the GenericSeries evaluation to work with correctly assigned array coordinates
We could probably change the SeriesParameter to always assign coordinates in SeriesParameter.data_array or are there places where we do not want that?
…into 691_spatial_series
|
great,... add a type hint,... get 9 unrelated warnings in the doc-build 😕 👎 |
Co-authored-by: Cagtay Fabry <43667554+CagtayFabry@users.noreply.github.com>
if unrelated just ignore and merge |
Co-authored-by: Cagtay Fabry <43667554+CagtayFabry@users.noreply.github.com>
|
Since we added a variable for the name of the dimension used by the |
Changes
Describe changes in this PR
Related Issues
Closes #691
Checks