You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I took a look at PR #7 and wanted to share some thoughts for consideration:
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.
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.
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 ENH : Added get_chunks to betweenness_centrality and create_iterables in utils #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.
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 the
Backendclass (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 the
NxReduceclass 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 the
NxMapclass, 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 ENH : Addedget_chunkstobetweenness_centralityandcreate_iterablesinutils#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 :)