Opened 11 months ago
Closed 10 months ago
#27491 closed defect (fixed)
deprecate parameter copy in networkx_graph
Reported by:  ghrajat1433  Owned by:  ghrajat1433 

Priority:  major  Milestone:  sage8.8 
Component:  graph theory  Keywords:  
Cc:  dcoudert  Merged in:  
Authors:  Rajat Mittal  Reviewers:  David Coudert 
Report Upstream:  N/A  Work issues:  
Branch:  cefb5fb (Commits)  Commit:  cefb5fb2659f7d5d7d9f21a1306e7703d76cbd75 
Dependencies:  Stopgaps: 
Description (last modified by )
Done in this ticket:
 deprecate parameter
copy
of methodnetworkx_graph
. This parameter was useful for the_nxg
backend that we don't have anymore.  remove remaining pieces of calls to
._backend._nxg
 remove deprecated class
NetworkXGraphDeprecated
. It should have been removed in #27166, but has been forgotten.
Change History (17)
comment:1 Changed 11 months ago by
 Branch set to u/ghrajat1433/27491_networkx_graph
 Owner changed from (none) to ghrajat1433
comment:2 Changed 11 months ago by
 Commit set to 1228fba5d93ba90f309e0f62e25ef96ff641d4e3
comment:3 Changed 11 months ago by
 Reviewers set to dcoudert
 Status changed from new to needs_review
comment:4 Changed 11 months ago by
This is a very good catch. However, what you propose to do is not what should be done.
In #27166, we removed deprecated networkx backends, but we forgot to update other parts.
 In file
src/sage/graphs/base/graph_backends.pyx
: remove classNetworkXGraphDeprecated
 in method
networkx_graph
: remove parametercopy
and update the code so that the method always returns a networkx graph.
 the name of this ticket will have to be updated accordingly
comment:5 Changed 11 months ago by
 Commit changed from 1228fba5d93ba90f309e0f62e25ef96ff641d4e3 to 3b48ef20b8e5c1556c787fec42156a19776bd24a
Branch pushed to git repo; I updated commit sha1. New commits:
3b48ef2  copy removed

comment:6 Changed 11 months ago by
 Description modified (diff)
 Summary changed from bug fix: default value true instead of false in networkx_graph method to bug fix: removal of copy parameter from networkx_graph
New commits:
3b48ef2  copy removed

comment:7 Changed 11 months ago by
 Description modified (diff)
 Priority changed from minor to major
comment:8 Changed 11 months ago by
I am not able to locate _backend._nxg in any backend file in the networkx_graph method
try: return self._backend._nxg except Exception:
can you point the location or does it always throw an error , if so we can remove it.
comment:9 Changed 11 months ago by
We don't have this backend anymore. This is a remaining of the very first releases of Sagemath that we have not fully removed yet (I forgot to do it in #27166).
You can also remove this from trees.pyx
# from networkx import MultiGraph # G = Graph(self.vertices) # cdef object XG = G._backend._nxg # # for i from 2 <= i <= self.vertices: # vertex1 = i  1 # vertex2 = self.current_level_sequence[i  1]  1 # XG.add_edge(vertex1, vertex2) # # return G # Currently, c_graph does not have all the same functionality as networkx. # Until it does, we can't generate graphs using the c_graph backend even # though it is twice as fast (for our purposes) as networkx.
comment:10 Changed 11 months ago by
 Commit changed from 3b48ef20b8e5c1556c787fec42156a19776bd24a to ba0855ab0eb0f4629312033299c9697aa8998114
Branch pushed to git repo; I updated commit sha1. New commits:
ba0855a  removed unnecessary code

comment:11 Changed 11 months ago by
 Reviewers changed from dcoudert to David Coudert
comment:12 Changed 10 months ago by
 Description modified (diff)
 Summary changed from bug fix: removal of copy parameter from networkx_graph to deprecate parameter copy in networkx_graph
I was a little to fast. To be clean, we should add a deprecation warning for parameter copy
.
Could you:
 put back parameter
copy
as input parameter, but without documentation. So def networkx_graph(self): + def networkx_graph(self, copy=True):
 and add the following at the very beginning of the method
+ if copy is not True: + deprecation(27491, "parameter copy is removed")
Then, in one year, we will completely remove this parameter.
Another small modification:
 N.add_nodes_from(self.vertices()) + N.add_nodes_from(self)
Method vertices
sorts vertices by default, and returns a list of vertices. Method add_nodes_from
can take an iterator as input. So we save the creation of the list.
I changed the description of this ticket to be consistent with what it does.
comment:13 Changed 10 months ago by
 Commit changed from ba0855ab0eb0f4629312033299c9697aa8998114 to cefb5fb2659f7d5d7d9f21a1306e7703d76cbd75
Branch pushed to git repo; I updated commit sha1. New commits:
cefb5fb  deprecated copy

comment:14 Changed 10 months ago by
deprecated the copy parameter.
comment:16 Changed 10 months ago by
 Milestone changed from sage8.7 to sage8.8
comment:17 Changed 10 months ago by
 Branch changed from u/ghrajat1433/27491_networkx_graph to cefb5fb2659f7d5d7d9f21a1306e7703d76cbd75
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
bug fix