-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds initial debug logging calls to _dispatchable #7300
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If adding debug logging will help with maintenance of this, its probably worth it. I typically use print statements for debugging and UserWarnings or exceptions for non-debugging "logging". But I don't use big complex software systems. So I'm approving this and will let more knowledgeable people decide how it is best set up.
@@ -523,6 +526,10 @@ def __call__(self, /, *args, backend=None, **kwargs): | |||
fallback_to_nx=self._fallback_to_nx, | |||
) | |||
# All graphs are backend graphs--no need to convert! | |||
_logger.debug( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to have an env variable, like NETWORKX_BACKEND_DEBUG
, and then only display these logging messages when this env variable is True
.
Or use _logger.info
instead of _logger.debug
if you think this msg should be displayed to the user every time an algorithm is dispatched to a backend implementation.
also, what would happen if a backend implementation calls another backend implementation? I don't think that internally used function calls also get logged, but should they get logged? For eg, the below parallel implementation calls another parallel function(this is not actually the case, in nx-parallel all_pairs_bellman_ford_path
calls the networkx's implementation of single_source_bellman_ford_path
, i just wanted to give an eg):
def all_pairs_bellman_ford_path(G, weight="weight", get_chunks="chunks"):
....
....
return [
(node, nxp.single_source_bellman_ford_path(G, node, weight=weight))
for node in node_chunk
]
....
so here the internally called function is single_source_bellman_ford_path
and I'd suggest that this internally used function should also be logged. And the user can define the level
(an env variable or something) of logging or debugging. LMK what u think.
Thank you :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can also add things like debug, caching, NETWORKX_AUTOMATIC_BACKENDS
, and other backend env variables to networkx config.. this might also make it easier to implement @eriknw’s idea of bundling all the backend environment variables and ignoring them.
[Anthony] adding log levels(DEBUG
, INFO
, WARN
, ERROR
, etc.) (ref. docs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this in for now, we can reiterate on the logging bits in later PRs too.
This PR adds debug logging for dispatched NetworkX functions.
Debug logging is enabled using the standard python
logging
API:Debug messages are not shown when functions are not dispatched: