Opened 11 years ago

Closed 11 years ago

#11592 closed defect (fixed)

Improvements to units convert function

Reported by: Eviatar Bach Owned by: Burcin Erocal
Priority: major Milestone: sage-4.7.2
Component: symbolics Keywords: units
Cc: Eviatar Bach Merged in: sage-4.7.2.alpha1
Authors: Eviatar Bach Reviewers: Burcin Erocal
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Eviatar Bach)

Previously, there were some problems with the unit convert function, namely that variables were not allowed. The following now works:

sage: sage.symbolic.units.convert(50 * x * units.area.square_meter, units.area.acre)
(1953125/158080329*x)*acre

As well, previously units were sometimes "mixed" with the returned symbolic (see http://ask.sagemath.org/question/641/radian-degree-conversion). Now, the following works:

sage: sage.symbolic.units.convert(cos(50) * units.angles.radian, units.angles.degree)
(180*cos(50)/pi)*degree

Attachments (1)

11592.patch (4.2 KB) - added by Eviatar Bach 11 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 years ago by Eviatar Bach

Description: modified (diff)

comment:2 Changed 11 years ago by Eviatar Bach

Status: newneeds_review

comment:3 Changed 11 years ago by Eviatar Bach

Cc: Eviatar Bach added

comment:4 Changed 11 years ago by Eviatar Bach

Milestone: sage-4.7.1sage-4.7.2

comment:5 Changed 11 years ago by Burcin Erocal

Authors: Eviatar Bach
Keywords: units added
Reviewers: Burcin Erocal
Status: needs_reviewneeds_work
Type: enhancementdefect

Thanks for the patch. Looks good to me, though a few minor changes are necessary before I can switch to positive review.

The patch bot shows failing tests in sage/symbolic/expression.pyx. While fixing those, please update the commit message to contain something meaningful in the first line. Deleting the first 2 lines of the current message should suffice.

Changed 11 years ago by Eviatar Bach

Attachment: 11592.patch added

comment:6 Changed 11 years ago by Eviatar Bach

Great! New patch uploaded.

comment:7 Changed 11 years ago by Eviatar Bach

It there something wrong with the patchbot? My local copy is passing all the tests.

comment:8 Changed 11 years ago by Burcin Erocal

Status: needs_workneeds_review

comment:9 Changed 11 years ago by Burcin Erocal

Status: needs_reviewpositive_review

comment:10 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-4.7.2.alpha1
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.