add scatter3d docs and fix colorbar title #5418
Conversation
|
Looks good. Just one comment, there's little point adding user docstrings to plotting classes since no one directly uses them. |
|
Where should they be added then @philippjfr. I am a new contributor to HoloViews. Currently Scatter3D does not show any docstring and I am not capable of navigating the code yet. |
|
The element is the right place. So this PR is totally fine, just doesn't have to be on the plotting class too. |
|
I don't understand. Could you be more specific? |
|
Oh wait sorry, this PR is only on the plotting classes. It'll have to be in the holoviews/element directory. |
|
For me this is the element. But that docstring is not shown when I use
It is shown when I hover though
|
| Example - Matplotlib | ||
| -------------------- | ||
|
|
||
| .. code-block:: |
There was a problem hiding this comment.
Previously I have put code blocks into >>> lines. But they are hard to copy and use directly. VS Code actually renders .. code-block:: nicely. So this is better to use.
Codecov Report
@@ Coverage Diff @@
## main #5418 +/- ##
=======================================
Coverage 88.23% 88.24%
=======================================
Files 302 302
Lines 62234 62260 +26
=======================================
+ Hits 54914 54939 +25
- Misses 7320 7321 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Okay, we'll have to take a look at hv.help then. Don't love the output in any case. |
MarcSkovMadsen
changed the title
| elif title.startswith("dim('") and title.endswith("')"): | ||
| title = title[5:-2] | ||
| else: | ||
| title = title[1:-1] |
There was a problem hiding this comment.
I don't know why title[1:-1] is a valid value from time to time. But it was there before my time.
There was a problem hiding this comment.
Probably because of something like this title = "'An awesome title'"
|
I don't plan to do more here. I hope someone with the knowledge will see they can quickly finalize and it will be a big step forward for |
|
@philippjfr Looks close to being ready for the imminent release and I'm happy to merge once the tests are green. From the look of it, the unit tests need updating but I am confused as to why the expected strings are now dictionaries? |
|
@MarcSkovMadsen , will you be able to address those test failures (to do with the titles)? It would be great to get this merged. |
|
As stated in the comments I dont know how to fix this. I need help. |
|
I'm not sure I am following you, didn't my last commit finalize your tests? Or are you talking about another comment? |
|
Sorry and thanks. I was replying to @jbednar old comment. |
|
The failed mac os test on Python 3.11 yesterday was because a debug version of Python was being installed: This should be fixed now. See conda-forge/python-feedstock#597 (comment) |
This addresses #5416 and #5423 a bit. Closes #4952