Opened 4 years ago
Last modified 4 years ago
#18481 needs_info enhancement
Allow matrix subdivide to optionally return a copy
Reported by:  rbeezer  Owned by:  

Priority:  minor  Milestone:  sage6.9 
Component:  linear algebra  Keywords:  matrix subdivide copy 
Cc:  tgagne  Merged in:  
Authors:  Reviewers:  Vincent Delecroix  
Report Upstream:  N/A  Work issues:  
Branch:  u/rbeezer/matrixcopyonsubdivide (Commits)  Commit:  e974b54136e531ca6040455a1f5af72dc0a3a234 
Dependencies:  Stopgaps: 
Description
The .subdivide
matrix method acts in place. There are situations when a copy, with the subdivisions, is desirable. An optional keyword, copy
will provide this option.
Change History (10)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
 Branch set to u/rbeezer/matrixcopyonsubdivide
 Commit set to e974b54136e531ca6040455a1f5af72dc0a3a234
New commits:
e974b54  18481: optionally copy matrix when adding subdivisions

comment:3 Changed 4 years ago by
 Status changed from new to needs_review
comment:4 Changed 4 years ago by
 Cc tgagne added
comment:5 Changed 4 years ago by
Followon ticket, with adjustments to the sage_input()
command for matrices, is at #18550.
comment:6 Changed 4 years ago by
 Milestone changed from sage6.8 to sage6.9
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to needs_info
Argh. There is always a choice between inplace
and copy
argument... we should definitely fix the standard for Sage. A very rough search gives
$ grep "^[[:space:]]*def [azAZ09_]*(" $(find . name "*.py")  grep "copy=" ./matrix/matrix_space.py: def __call__(self, entries=None, coerce=True, copy=True, sparse = False): ./matrix/matrix_space.py: def matrix(self, x=0, coerce=True, copy=True): ./combinat/combinat.py: def __init__(self, l, copy=True): ./combinat/designs/group_divisible_designs.py: def __init__(self, points, groups, blocks, G=None, K=None, lambd=1, check=True, copy=True,**kwds): ./combinat/designs/bibd.py: def __init__(self, points, blocks, K=None, lambd=1, check=True, copy=True,**kwds): ./combinat/designs/bibd.py: def __init__(self, points, blocks, k=None, lambd=1, check=True, copy=True,**kwds): ./graphs/generic_graph.py: def networkx_graph(self, copy=True): ./modules/free_module.py: def _element_constructor_(self, x, coerce=True, copy=True, check=True): ./geometry/newton_polygon.py: def vertices(self, copy=True):
against
e$ grep "^[[:space:]]*def [azAZ09_]*(" $(find . name "*.py")  grep "inplace=" ./combinat/root_system/dynkin_diagram.py: def relabel(self, relabelling, inplace=False, **kwds): ./combinat/ordered_tree.py: def normalize(self, inplace=False): ./combinat/ordered_tree.py: def dendrog_normalize(self, inplace=False): ./combinat/cluster_algebra_quiver/cluster_seed.py: def mutate(self, sequence, inplace=True): ./combinat/cluster_algebra_quiver/quiver.py: def principal_extension(self, inplace=False): ./combinat/cluster_algebra_quiver/quiver.py: def mutate(self, data, inplace=True): ./combinat/designs/incidence_structures.py: def relabel(self, perm=None, inplace=True): ./combinat/yang_baxter_graph.py: def relabel_vertices(self, v, relabel_operator, inplace=True): ./combinat/yang_baxter_graph.py: def relabel_edges(self, edge_dict, inplace=True): ./combinat/yang_baxter_graph.py: def relabel_vertices(self, v, inplace=True): ./graphs/digraph.py: def reverse_edge(self, u, v=None, label=None, inplace=True, multiedges=None): ./graphs/digraph.py: def reverse_edges(self, edges, inplace=True, multiedges=None): ./graphs/generic_graph.py: def subgraph(self, vertices=None, edges=None, inplace=False, ./graphs/generic_graph.py: def _subgraph_by_deleting(self, vertices=None, edges=None, inplace=False, ./graphs/generic_graph.py: def random_subgraph(self, p, inplace=False): ./graphs/generic_graph.py: def relabel(self, perm=None, inplace=True, return_map=False, check_input = True, complete_partial_function = True): ./graphs/generic_graph.py:def graph_isom_equivalent_non_edge_labeled_graph(g, partition=None, standard_label=None, return_relabeling=False, return_edge_labels=False, inplace=False, ignore_edge_labels=False): ./modular/arithgroup/arithgroup_perm.py: def relabel(self, inplace=True):
What do you think? Is it worth opening a thread on sagedevel?
Vincent
comment:7 Changed 4 years ago by
Dear Vincent,
Thanks for looking at this, and for the excellent grep'ing skills. Sorry I didn't get to this right away.
Well, it looks like inplace
wins by frequency. But a discussion on sagedevel might prove interesting. I'm in no hurry. And maybe others can point to the discussion later (or have it go in the developer's guide now).
I'll be at Sage Days next week (arriving a bit late), so that would be a good time to finish this one off if there is a consensus.
Thanks, Rob
comment:8 Changed 4 years ago by
Hi Rob,
As I actually said on sagedevel, I would like to keep the following semantics for inplace/copy
:
inplace
: modify (or not) the object, likel.remove(1)
is an inplace modification on a listl
;copy
: whether the created object will hold the argument or make a copy of them, likematrix(ZZ, data=[1,2,3,4], copy=False)
.
For the purpose of this ticket, this would be an inplace
.
Vincent
comment:9 followup: ↓ 10 Changed 4 years ago by
Dear Vincent,
Back from vacation and now at Sage Days 68 for a few days before classes begin.
What do you think of Volker's suggestion that there could be a .subdivided()
method, which would return a changed copy? That sounds like a good solution to me.
Rob
comment:10 in reply to: ↑ 9 Changed 4 years ago by
Replying to rbeezer:
What do you think of Volker's suggestion that there could be a
.subdivided()
method, which would return a changed copy? That sounds like a good solution to me.
+1
I'd like to have the
sage_input()
command preserve the subdivisions of matrix. So the output ofsage_input(A)
could be something likepresuming the copy option proposed here was functional. So this ticket is a precursor to the upcoming
sage_input
ticket. Work on the present ticket is close to finished.