Opened 10 years ago

Closed 10 years ago

# [with patch, with positive review] conversion of sage matrices to mathematica is just completely totally broken

Reported by: Owned by: was TimothyClemans major sage-2.10.3 interfaces

### Description

We have

```sage: n = matrix(QQ, 3, range(9))
sage: n._mathematica_init_()
'{{0},{1},{2},{3},{4},{5},{6},{7},{8}}'
```

but we should have

```sage: n = matrix(QQ, 3, range(9))
sage: n._mathematica_init_()
'{{0,1,2},{3,4,5},{6,7,8}}'
```

### comment:1 Changed 10 years ago by mabshoff

• Milestone changed from sage-2.10 to sage-2.9.1

### comment:2 Changed 10 years ago by mabshoff

• Summary changed from conversion of sage matrices to mathematica is just completely totally broken to [with patch, needs review] conversion of sage matrices to mathematica is just completely totally broken

### comment:3 Changed 10 years ago by TimothyClemans

• Owner changed from was to TimothyClemans
• Status changed from new to assigned

### comment:4 Changed 10 years ago by ncalexan

• Summary changed from [with patch, needs review] conversion of sage matrices to mathematica is just completely totally broken to [with patch, with negative review] conversion of sage matrices to mathematica is just completely totally broken

This will not work, since it does not appear to be recursive. For example:

```sage: var('x, y, z, b')
(x, y, z, b)
sage: f = sin(x^2) + y^z
sage: f
y^z + sin(x^2)
sage: f._mathematica_init_()
'(Sin[(x) ^ (2)]) + ((y) ^ (z))'
sage: M = matrix(1, 2, [f, f^2]); M
[    y^z + sin(x^2) (y^z + sin(x^2))^2]
```

Also, please post a unified patch making it easy to see just the total changes.

### Changed 10 years ago by was

(this one by William and Timothy) apply this patch right after applying 1382_2.patch

### comment:5 Changed 10 years ago by was

• Summary changed from [with patch, with negative review] conversion of sage matrices to mathematica is just completely totally broken to [with patch, needs new review] conversion of sage matrices to mathematica is just completely totally broken

### comment:6 Changed 10 years ago by ncalexan

• Summary changed from [with patch, needs new review] conversion of sage matrices to mathematica is just completely totally broken to [with patch, with positive review] conversion of sage matrices to mathematica is just completely totally broken

Patch looks good, I say apply.

Is _mathematica_init_ guaranteed not to require Mathematica? If not, some more doctests need to be declared optional. Otherwise, looks good.

### comment:7 Changed 10 years ago by was

Is _mathematica_init_ guaranteed not to require Mathematica?

Yes. It returns a string is must not call Mathematica.

William

### comment:8 Changed 10 years ago by mabshoff

Somebody ought to clue be in

• which patches to apply in which order (the comments are unclear)
• if all problems regarding optional doctests are solved, i.e. the last comment by William

Cheers,

Michael

### comment:9 Changed 10 years ago by was

Michael. Apply 1382_2.patch then 1382_5-part2of2.patch. And the comment about optional is not applicable since _mathematica_init_ should never depend on mathematica.

### comment:10 Changed 10 years ago by TimothyClemans

William's patch seems to be editing 1382_5.patch. task_1832_2.patch depends on ticket_1382.patch and it doesn't have lines like "return mathematica.mathematica(tuple(self))" which are in 1382_5.patch and show up in red on http://trac.sagemath.org/sage_trac/attachment/ticket/1382/1382_5-part2of2.patch

### comment:11 Changed 10 years ago by mabshoff

• Summary changed from [with patch, with positive review] conversion of sage matrices to mathematica is just completely totally broken to [with patch, with positive review, needs cleaned up patches] conversion of sage matrices to mathematica is just completely totally broken

The patches at this ticket are a mess either way: For task_1832_2.patch I need to ticket_1382.patch. But at that point 1382_5-part2of2.patch doesn't apply cleanly. So, in conclusion: Please post a single patch that has all the changes and that applies cleanly against 2.10.2. To do that merge the fixes back by hand, do not just post a bundle with all patches applied.

Cheers,

Michael

### comment:12 Changed 10 years ago by was

I made a typo. Simply apply

• 1382_5.patch
• 1382_5-part2of2.patch.

That's it. 2 patches. They should not be flattened.

William

### comment:13 Changed 10 years ago by mabshoff

• Resolution set to fixed
• Status changed from assigned to closed

Merged 1382_5.patch and 1382_5-part2of2.patch in Sage 2.10.3.alpha0. Thanks William for the clarification.

### comment:14 Changed 10 years ago by mabshoff

• Summary changed from [with patch, with positive review, needs cleaned up patches] conversion of sage matrices to mathematica is just completely totally broken to [with patch, with positive review] conversion of sage matrices to mathematica is just completely totally broken
Note: See TracTickets for help on using tickets.