-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Remove vestigial debug print statement in walk_head_nodes
#10718
Conversation
I realize that it's a really small change, but all the reformatting makes this PR kind of hard to follow and is going to make the history really confusing in the future. (I had to use a text search to find the place where the print statement was removed in the commit.) We don't have automatic processes for formatting cython, so if there's a good solution that you're aware of, it would be useful to add it. If it's not, then I would prefer if this kind of PR didn't do any unrelated reformatting, or at least not beyond a few lines at most. What does everyone think? |
Ah, I didn't notice all the whitespace changes on my end. I can revert them, not an issue. That said, the GitHub diff interface does let one hide whitespace changes: Click on the gear icon at the top of the Files Changed page, check "Hide whitespace" and hit Apply. I don't have a specific formatter for Cython, but my editor is configured to automatically trim trailing whitespaces (which is what happened here). |
Agreed! As long as we don't have a standard way of formatting Cython, I'd avoid making too many formatting changes in parts of the code that would otherwise be untouched by a PR. Mostly for |
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.
The graph.pyx
stuff is not currently used by anyone or anything, as far as I know. There's actually plenty of other print
statements that should be cleaned up in this same file, or at least put behind some debug
variable or something.
The fact that that didn't happen yet though, makes me wonder whether this was still a WIP that still required some further debugging & experimentations first...
I only see two other In any event, I do think it makes sense to merge the proposed change as it will eliminate a potential bottleneck in the future, especially given that the GIL lock was being acquired in a graph traversal method. |
Yea, let's just remove the other |
* `graph`: Remove vestigial debug print statement in `walk_head_nodes` * Revert whitespace changes * Remove more debug print statements
Description
Removes one of the last, potential GIL-Refnanny related regressions.
Types of change
Bug fix
Checklist