#1382 closed defect (fixed)
[with patch, with positive review] conversion of sage matrices to mathematica is just completely totally broken
Reported by: | was | Owned by: | TimothyClemans |
---|---|---|---|
Priority: | major | Milestone: | sage-2.10.3 |
Component: | interfaces | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
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}}'
Attachments (7)
Change History (21)
comment:1 Changed 10 years ago by
- Milestone changed from sage-2.10 to sage-2.9.1
Changed 10 years ago by
comment:2 Changed 10 years ago by
- 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
Changed 10 years ago by
Changed 10 years ago by
Changed 10 years ago by
comment:3 Changed 10 years ago by
- Owner changed from was to TimothyClemans
- Status changed from new to assigned
Changed 10 years ago by
comment:4 Changed 10 years ago by
- 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
Changed 10 years ago by
Changed 10 years ago by
(this one by William and Timothy) apply this patch right after applying 1382_2.patch
comment:5 Changed 10 years ago by
- 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
- 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
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
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
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
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
- 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
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
- 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
- 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
This will not work, since it does not appear to be recursive. For example:
Also, please post a unified patch making it easy to see just the total changes.