networkx / networkx/nx-parallel
Refactoring and Simplification Suggestions(based on PR #7)
Nobody has claimed this yet.
- 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:
-
Code Organization in
external.pyandmisc.py:
I suggest keeping most of theBackendclass (inexternal.py) and moving theoptional_packagefunction (inmisc.py) toexternal.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 theoptional_packagefunction with theBackendclass. Let me know if you see any potential drawbacks to this approach. -
Exclusion of
NxReduceinpartition.py:
I propose excluding theNxReduceclass frompartition.py. Since most of its functions are one-liners, it might be more intuitive for new contributors to write these directly instead of using theNxReduceclass. Additionally, this adjustment could enhance code readability, especially for those new to the library. I couldn't identify any scenarios where we'd preferNxReduceover a simplereduce(lambda ...). Please share your insights on this. -
Refactoring the
NxMapclass :
For theNxMapclass, 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 oneNxMapclass, like if we want to have theget_chunksparameter(in PR #29) that lets the user define their ownnode_chunksthen we would not be requiringchunks, but we might requirecreate_iterablesfunction independently(depending on the algo). Please let me know your thoughts on this.
Thank you :)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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