plumbs set_property from Visualizer and through the zmqserver

[RussTedrake]

note: i did not set path.tolower() in set_property, despite the other senders having it. because i think that is a mistake.

---

This change is 

[codecov-commenter]

Codecov Report

Merging #67 into master will decrease coverage by 0.51%. The diff coverage is 64.28%.

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
- Coverage   89.64%   89.13%   -0.52%     
==========================================
  Files           8        8              
  Lines         724      736      +12     
==========================================
+ Hits          649      656       +7     
- Misses         75       80       +5     

Impacted Files	Coverage Δ
src/meshcat/commands.py	87.75% <50.00%> (-7.37%)	:arrow_down:
src/meshcat/visualizer.py	78.51% <83.33%> (-0.13%)	:arrow_down:

---

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 bb4365a...7e1dc2e. Read the comment docs.

[RussTedrake]

---

src/meshcat/visualizer.py, line 179 at r1 (raw file):

Previously, rdeits (Robin Deits) wrote…

(copied from email) This should use `self.path` rather than taking a `path` string argument. To set the property of an object in the scene, you'd do: ```py vis["foo"]["bar"].set_property(key, value) ``` just like `set_transform` and `set_object`.

thanks. i had missed that. but

vis["Background"].set_property(...)

doesn't work because the path ends up being /meshcat/Background. There doesn't seem to be a good way to set_transform/object/property for elements at the root of the tree, yet?

What do you think? I don't actually know the motivation for prepending the meshcat to the path. My first choice of a solution would be to simply remove that. I don't think it would impact any users (unless they were writing javascript that was depending on that naming convention).

Alternatively, I could have optional path_override arguments to set_transform/object/property, etc, but that seems gross. Or I could have a special handler for ".." in Visualizer.__getitem__. I don't like that much either.

Let me know what you think? Perhaps I'm just missing something (again)!

[RussTedrake]

---

src/meshcat/visualizer.py, line 179 at r1 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

thanks. i had missed that. but ``` vis["Background"].set_property(...) ``` doesn't work because the path ends up being `/meshcat/Background`. There doesn't *seem* to be a good way to set_transform/object/property for elements at the root of the tree, yet? What do you think? I don't actually know the motivation for prepending the `meshcat` to the path. My first choice of a solution would be to simply remove that. I don't think it would impact any users (unless they were writing javascript that was depending on that naming convention). Alternatively, I could have optional `path_override` arguments to set_transform/object/property, etc, but that seems gross. Or I could have a special handler for ".." in `Visualizer.__getitem__`. I don't like that much either. Let me know what you think? Perhaps I'm just missing something (again)!

ftr -- i realized a moment after sending, that probably the reason for the default meshcat in the path was so that the simple example, with only a single transform/object works.

[rdeits]

You can do vis["/Background"] to access a path from the root of the tree rather than a relative path. And yeah, you're right that the default "/meshcat" path is there to make the default visualizer work without any path arguments.

On Sun, Jul 26, 2020, 08:22 Russ Tedrake notifications@github.com wrote:

---

src/meshcat/visualizer.py, line 179 at r1 https://reviewable.io/reviews/rdeits/meshcat-python/67#-MD9XxOIQ_qPxtLTW_R0-r1-179:-MDA3ZVm8YW2nu20ZqjU:bp4zm2h (raw file https://github.com/rdeits/meshcat-python/blob/7bf0ce78eab60c0dc6c58322cb8e8643d2580a9a/src/meshcat/visualizer.py#L179): Previously, RussTedrake (Russ Tedrake) wrote…

thanks. i had missed that. but

vis["Background"].set_property(...)

doesn't work because the path ends up being /meshcat/Background. There doesn't seem to be a good way to set_transform/object/property for elements at the root of the tree, yet?

What do you think? I don't actually know the motivation for prepending the meshcat to the path. My first choice of a solution would be to simply remove that. I don't think it would impact any users (unless they were writing javascript that was depending on that naming convention).

Alternatively, I could have optional path_override arguments to set_transform/object/property, etc, but that seems gross. Or I could have a special handler for ".." in Visualizer.getitem. I don't like that much either.

Let me know what you think? Perhaps I'm just missing something (again)!

ftr -- i realized a moment after sending, that probably the reason for the default meshcat in the path was so that the simple example, with only a single transform/object works.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/rdeits/meshcat-python/pull/67#issuecomment-663981320, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEQQDSMHIMZFLMBL4EBLNLR5QNZXANCNFSM4PHT34KQ .