Fixed broken URL in help output #1051
Conversation
jlstevens
force-pushed
the
help_url_fix
branch
from
4e85cd1
to
56d6f4a
Compare
|
I'm a little surprised that the tests passed...maybe this output is being sent to the pager and therefore isn't captured? |
|
Yes, I think that's it. One issue I have is that custom Elements will also point to non-existent entries in the elements tutorial. Got any ideas to disable that? |
|
I agree that is an issue though not a particularly major one. My only suggestion is to suggest the help URL only if the element is one of the ones we offer. Do we have such a list? Maybe the top level |
|
Could probably just iterate over the |
|
I suppose so given that seaborn elements aren't in that tutorial anyway. Should I really do the same for Containers or can I just ignore the possibility of custom Containers? I doubt people will want to build new containers often... |
Fine to ignore right now I think. |
|
Added filtering of elements as suggested:
Versus a core element:
Note this PR isn't quite ready to merge. I might as well fix the issue about showing help for the active backend while I am working on this code. |
| @@ -25,6 +25,7 @@ | |||
| from .operation import ElementOperation, MapOperation, TreeOperation # noqa (API import) | |||
| from .element import * # noqa (API import) | |||
| from . import util # noqa (API import) | |||
| from .element import __all__ as elements_list | |||
There was a problem hiding this comment.
That doesn't seem right, it should at least filter for objects of type Element.
There was a problem hiding this comment.
I know hv.element.__all__ filters for ViewableElement but I think that includes some other types as well, e.g. Overlays. So maybe just tighten that check to use Element instead of ViewableElement.
There was a problem hiding this comment.
The problem is that __all__ contains strings (which I want) but doing tighter subclass checks requires a list of those classes. I could iterate over locals but I find that pretty ugly. What would you suggest?
There was a problem hiding this comment.
I'd change line 99 of hv.element.__init__ to:
return issubclass(obj, Element)
There was a problem hiding this comment.
Done! I was hesitant in case anything was expecting to find ViewableElements...
There was a problem hiding this comment.
Appears you were right being hesitant, but it really shouldn't be relying on importing Overlay via hv.element.
|
Ok. If the tests pass I think this PR is ready for final review/merge. |
|
Looks good, merging. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This PR addresses #978. The code change is trivial but it is likely that the test data will need updating.