Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to units convert function #11592

Closed
eviatarbach opened this issue Jul 13, 2011 · 8 comments
Closed

Improvements to units convert function #11592

eviatarbach opened this issue Jul 13, 2011 · 8 comments

Comments

@eviatarbach
Copy link

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

CC: @eviatarbach

Component: symbolics

Keywords: units

Author: Eviatar Bach

Reviewer: Burcin Erocal

Merged: sage-4.7.2.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/11592

@eviatarbach

This comment has been minimized.

@burcin
Copy link

burcin commented Jul 20, 2011

Author: Eviatar Bach

@burcin
Copy link

burcin commented Jul 20, 2011

comment:5

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.

@burcin
Copy link

burcin commented Jul 20, 2011

Reviewer: Burcin Erocal

@burcin
Copy link

burcin commented Jul 20, 2011

Changed keywords from none to units

@eviatarbach
Copy link
Author

comment:6

Attachment: 11592.patch.gz

Great! New patch uploaded.

@eviatarbach
Copy link
Author

comment:7

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

@jdemeyer
Copy link

jdemeyer commented Aug 3, 2011

Merged: sage-4.7.2.alpha1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants