Opened 11 years ago

Closed 9 years ago

#9759 closed enhancement (duplicate)

Addition of SI prefixes capabilities to the units module

Reported by: cousteau Owned by: burcin
Priority: trivial Milestone: sage-duplicate/invalid/wontfix
Component: symbolics Keywords: sd40.5
Cc: rbeezer, was Merged in:
Authors: Reviewers: Javier Mora, Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

Although the units module already has a si_prefixes component, it's not very convenient, since you have to do units.si_prefixes.nano*units.mass.gram, and you get something like "gram*nano" that doesn't look very well. This ticket is a modification that adds properties named as the components of units.si_prefixes, so that calling something like units.mass.gram.nano will create a units.mass.nanogram element and return it.

Duplicate of #9778.

Attachments (1)

trac_9759_si_prefixes.patch (1.6 KB) - added by cousteau 11 years ago.
Patch that adds components of units.si_prefixes as properties on the UnitExpression? class

Download all attachments as: .zip

Change History (10)

Changed 11 years ago by cousteau

Patch that adds components of units.si_prefixes as properties on the UnitExpression? class

comment:1 Changed 11 years ago by rbeezer

  • Cc rbeezer added

comment:2 Changed 11 years ago by cousteau

  • Status changed from new to needs_review

comment:3 Changed 11 years ago by cousteau

  • Cc was added

comment:4 Changed 10 years ago by burcin

  • Milestone set to sage-4.7.1

The patch looks good to me. It is a hack and I am not really happy with the use of sage_eval(), but this seems to be used everywhere in sage/symbolic/units.py. I'm ready to give a positive review, though it would be better if somebody who actually uses this module comments on the improvement.

Why does the new function name start with an underscore? Wouldn't it be easier to find it if was just named si_prefix()?

comment:5 follow-up: Changed 10 years ago by cousteau

The truth is that this patch was created as a suggestion done in #sage-devel; it was done pretty fast and probably not elegantly. I later submitted another patch with a better implementation (see ticket #9778), which also added the basics for LaTeX representation of the units.

(I also made the alternate "metrology" module on #9763, which also has LaTeX and units support)

comment:6 in reply to: ↑ 5 ; follow-up: Changed 9 years ago by kcrisman

  • Keywords sd40.5 added
  • Status changed from needs_review to needs_work

I agree with Burcin that this is a bit hackish.

The truth is that this patch was created as a suggestion done in #sage-devel; it was done pretty fast and probably not elegantly. I later submitted another patch with a better implementation (see ticket #9778), which also added the basics for LaTeX representation of the units.

So should this be closed in favor of #9778?

Also,

def _add_si_property_(prefix): 
 	    setattr(UnitExpression, prefix, property(lambda self: self._si_prefix_(prefix))) 
 
for prefix in unitdict['si_prefixes']: 
     _add_si_property_(prefix) 

seems to be missing a doctest in the underscore function.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 9 years ago by cousteau

Replying to kcrisman:

So should this be closed in favor of #9778?

I think it'd be a good idea.

comment:8 in reply to: ↑ 7 Changed 9 years ago by kcrisman

  • Authors Javier Mora deleted
  • Milestone changed from sage-5.1 to sage-duplicate/invalid/wontfix
  • Reviewers set to Javier Mora, Karl-Dieter Crisman
  • Status changed from needs_work to positive_review

So should this be closed in favor of #9778?

I think it'd be a good idea.

Okay, I'll make a comment there to that effect.

comment:9 Changed 9 years ago by jdemeyer

  • Description modified (diff)
  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.