Rename _dispatch to _dispatchable
#7193
Conversation
|
This addresses one of the issues @JDLH raised in #7189 and goes with the suggestion by @rlratzel from #7189 (comment) |
|
Should the name remain prefixed with an underscore forever? I know very little about backend dispatching in NetworkX, but I think I remember reading that they underscore prefix was to signal that this decorator being preliminary and in flux. If this change is "one last time", then maybe the name is no longer preliminary or in flux? |
Definitely not - we should have a discussion on the dispatching roadmap, which I won't dump so as not to distract from the PR.
I will say however that this is likely not the last change that would motivate removing the underscore! |
|
To clarify, when I said "consider changing the name one last time to |
There was a problem hiding this comment.
Personally, I like the proposed name change. I feel that it better captures that the "dispatchability" is a property of the decorated function, rather than some internal indirection that impacts the actual networkx implementation. This should hopefully help alleviate some of the concerns for readers of the source code highlighted in #7189 .
@JDLH what do you think - is the proposed name change more clear IYO?
|
@rossbar I think "dispatchable" is an improvement on the previous name "dispatch". It has the advantage that, as you say, it reads like a property of the decorated function. It has the disadvantage of being opaque to most readers about what that property actually means. The word "dispatchable" does not by itself quite tell most readers what they need to know. Thus I think the new name needs to be paired with an explanation of the decorator, under the name "dispatchable", in the function reference documentation. When I was looking for an explanation of this decorator, the function reference is where I searched for the name "_dispatch". There was no text under that heading. I suggest that the explanation include a message to readers who don't care about backends, and are not using networkx as a front end to other libraries, that this decorator is of no concern to them. Then it should have a reference to the place in the documentation where dispatching and backends are explained. Maybe after that it has the normal reference documentation about the decorator, its parameters, its effects, etc. A lot of this reference documentation is presently located in the module utils/backends.py. |
Agreed - the renaming here is certainly not sufficient to address #7189 on its own. There should definitely be a very visible and easily discoverable reference with the explanation you describe. In addition to reference docs, it might also be worth adding a term to the glossary and using the |
|
Great 🚀 ! Yeah, docs should be updated too, but they should be updated in a separate PR. A substantial change (updating docs) shouldn't be buried with the large but simple change in this PR. And online docs for And also |
Co-authored-by: Dan Schult <dschult@colgate.edu>
How do people like the name
_dispatchable? Once there is better documentation for it (for users, backend developers, and networkx developers), then I think we should consider changing the name one last time todispatchable.