networkx / networkx/nx-parallel

Refactoring and Simplification Suggestions(based on PR #7)

Open
#30 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Infrastructure
Dominant language
Python
Stars
71
Forks
37
PR merge metrics
No merged PRs in 30d

Description

I took a look at PR #7 and wanted to share some thoughts for consideration:

  1. Code Organization in external.py and misc.py:
    I suggest keeping most of the Backend class (in external.py) and moving the optional_package function (in misc.py) to external.py. My reasoning is that, we probably want to have networkx and joblib as our main dependencies and we would probably be just using other packages for joblib's backends, so it seems logical to have the optional_package function with the Backend class. Let me know if you see any potential drawbacks to this approach.

  2. Exclusion of NxReduce in partition.py:
    I propose excluding the NxReduce class from partition.py. Since most of its functions are one-liners, it might be more intuitive for new contributors to write these directly instead of using the NxReduce class. Additionally, this adjustment could enhance code readability, especially for those new to the library. I couldn't identify any scenarios where we'd prefer NxReduce over a simple reduce(lambda ...). Please share your insights on this.

  3. Refactoring the NxMap class :
    For the NxMap class, I think we would have sufficient instances where it would be useful. But, maybe it would be better to have 2 independent functions(excluding __call__, bcoz one-line function) instead of one NxMap class, like if we want to have the get_chunks parameter(in PR #29) that lets the user define their own node_chunks then we would not be requiring chunks, but we might require create_iterables function independently(depending on the algo). Please let me know your thoughts on this.

Thank you :)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review PR #7 and the related PR #29 first, then compare the proposed responsibilities in external.py, misc.py, and partition.py, including Backend, optional_package, NxReduce, and NxMap. Done requires agreement on the refactoring plan and its intended API changes before implementation.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.