Update CSM tutorial

[vhirtham]

Changes

Updates the tutorial of the CSM.

Checks

• updated CHANGELOG.rst
• updated tests
• updated doc/
• update example/tutorial notebooks
• update manifest file

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[github-actions]

Unit Test Results

2 152 tests  ±0   2 152 ✔️ ±0   2m 31s ⏱️ -3s
       1 suites ±0          0 💤 ±0 
       1 files   ±0          0 ❌ ±0 

Results for commit 880149b. ± Comparison against base commit e36d678.

♻️ This comment has been updated with latest results.

[codecov]

Codecov Report

Merging #767 (880149b) into master (ceaa4c5) will decrease coverage by 0.06%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #767      +/-   ##
==========================================
- Coverage   96.83%   96.77%   -0.07%     
==========================================
  Files          88       88              
  Lines        6009     6014       +5     
==========================================
+ Hits         5819     5820       +1     
- Misses        190      194       +4     

Impacted Files	Coverage Δ
weldx/transformations/cs_manager.py	98.60% <80.00%> (-0.19%)	⬇️
weldx/asdf/util.py	91.19% <0.00%> (-0.95%)	⬇️

---

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 e36d678...880149b. Read the comment docs.

[vhirtham]

I think we can also remove the first code cell:

# enable interactive plots on Jupyterlab with ipympl and jupyterlab-matplotlib installed
# %matplotlib widget

There are only 2 matplotlib plots left and their purpose is to show, that matplotlib is also supported. All the other plots are interactive k3d plots. What do you think?

[marscher]

Thank you for updating the tutorial. It was a joy reading 👍

I will commit some changes (e.g. the parameter enumeration).
Another thing I've noticed is that the example about subsystems uses unrealistic units for the torch position in relation to the robot head (2 mm). This is perhaps a minor detail, but I guess we should change it to "cm" or increase the numbers to match a more realistic point of view. What do you think?

[marscher]

I've also added some padding for the csm.plot_graph method, so the labels of the subsystems are readable (they used to be truncated on the right side).

[marscher]

If this tutorial is meant to replace the "transformations_0*_tutorials", I'd not place this note into the introduction. At least do not make it the first sentence. Start with "The purpose of the CSM is...".

Would it be possible to link the CSM to its docstring inside the notebook?

Reply via ReviewNB

[vhirtham]

Not sure what you mean. The LCS tutorial will also get an update and this on isn't meant to replace anything. Its just an update :)

I can change the order though.

We can link the API documentation page.

[vhirtham]

Changed it.

[marscher]

a "member function" is a method, right?

I'd use a enumeration for the parameters, to make the section more readable.

Please explain the concept of a reference system before using it for the parameter explanation. E.g. show an equation to make things clear or just write one or two sentences what's the actual relation.

Reply via ReviewNB

[vhirtham]

done

[marscher]

The user does not need to know what the pattern of a "factory method" is. He or she needs to know that one can construct LCS directly by giving an Euler angle or coordinates.

Again it would be nice to link the docstrings to the actual methods here.

Reply via ReviewNB

[vhirtham]

yes, automatic links would be nice. I added one manually, but it is rather suboptimal. Unfortunately, code as text in links doesn't work well (bad visibility).

Removed mentioning of factory method.

[marscher]

The overview you've just given, right? The link contains ALL the details (which might be overwhelming, if this is the starting point).

Reply via ReviewNB

[vhirtham]

It's more about the construction methods that aren't explicitly mentioned here. I think it is a managable task for the user to look through the API doc ;)

[marscher]

not only internally, but the logical relation of the systems is a tree. Eventually you can elaborate on that (1-2 sentences).

Reply via ReviewNB

[vhirtham]

Think the rendered images should be enough, don't you think?

[marscher]

Nowadays, we call it Jupyter notebook :)

Reply via ReviewNB

[vhirtham]

;) changed it

[marscher]

Sorry, I do not understand the note. I was able to rename the specimen CS, even though the "TC" or "thermocouple" is still attached. Is this case not being catched by the CSM, or is the note obsolete?

Reply via ReviewNB

[vhirtham]

My bad. I changed the note to be more specific. It was about CSM instances that get merged into each other

[vhirtham]

Thank you for updating the tutorial. It was a joy reading 👍

I will commit some changes (e.g. the parameter enumeration). Another thing I've noticed is that the example about subsystems uses unrealistic units for the torch position in relation to the robot head (2 mm). This is perhaps a minor detail, but I guess we should change it to "cm" or increase the numbers to match a more realistic point of view. What do you think?

Yes, that sounds reasonable. The main focus here was to get everything "into the camera". However, this was more problematic when the static matplotlib plots were used. I think with k3d, we can experiment a bit more with the values and stretch everything out. Also, the xarray data points in the section about spatial data are barely visible in the plots. I think it might be a good idea to rename the data to "point cloud" instead of "sensor positions" (or whatever it was named) and generate a denser set of data with a list comprehension.

[vhirtham]

Tweaked some values as suggested. Take a look at the new plots and let me know what you think

[CagtayFabry]

great, thank you @vhirtham :)