Skip to content

Minor Python 2 cleanup#6219

Merged
jarrodmillman merged 1 commit intonetworkx:mainfrom
rossbar:redundancy-py3-dict-keys-as-sets
Nov 16, 2022
Merged

Minor Python 2 cleanup#6219
jarrodmillman merged 1 commit intonetworkx:mainfrom
rossbar:redundancy-py3-dict-keys-as-sets

Conversation

@rossbar
Copy link
Copy Markdown
Contributor

@rossbar rossbar commented Nov 16, 2022

Use dict.keys() for set operations rather than explicitly creating sets, as recommended in the comment.

Use dict.keys() for set operations rather than explicitly
creating sets.
@jarrodmillman jarrodmillman added this to the networkx-3.0 milestone Nov 16, 2022
@jarrodmillman jarrodmillman merged commit 1f03311 into networkx:main Nov 16, 2022
@dschult
Copy link
Copy Markdown
Member

dschult commented Nov 17, 2022

This change does work -- and you'd think it might be faster, but it looks like it is actually slower -- even for large sets.

Probably sticking with builtin python objects is faster than using the same tools via the NetworkX views. The collections.abc tools are nice, but not optimized for speed -- rather for ease of implementation I suspect.

Anyway, if anyone is thinking of propagating use of keys instead of sets throughout the library (and there are many other places it could be used), it probably isn't worth the effort. :{ It is probably slightly less readable too.

@rossbar
Copy link
Copy Markdown
Contributor Author

rossbar commented Nov 17, 2022

Thanks @dschult , I didn't bother to check the performance which was an oversight on my part!

In that case, I think the original is unquestionably better as it was a) shorter, b) more performant, and c) explicit (i.e. readers don't have to know that keys support set operations in order to understand the code). I'm going to undo the change less the comment.

rossbar added a commit to rossbar/networkx that referenced this pull request Nov 17, 2022
@dschult
Copy link
Copy Markdown
Member

dschult commented Nov 17, 2022

Getting rid of the comment is a Good Thing!
One more "ToDo" out of the codebase! :)

@rossbar
Copy link
Copy Markdown
Contributor Author

rossbar commented Nov 17, 2022

Getting rid of the comment is a Good Thing!

Just doing it the hard way ;)

MridulS pushed a commit to MridulS/networkx that referenced this pull request Feb 4, 2023
Python3 cleanup

Use dict.keys() for set operations rather than explicitly
creating sets.
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
Python3 cleanup

Use dict.keys() for set operations rather than explicitly
creating sets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants