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: |
Description (last modified by )
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)
Change History (10)
Changed 11 years ago by
comment:1 Changed 11 years ago by
- Cc rbeezer added
comment:2 Changed 11 years ago by
- Status changed from new to needs_review
comment:3 Changed 11 years ago by
- Cc was added
comment:4 Changed 10 years ago by
- 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: ↓ 6 Changed 10 years ago by
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: ↓ 7 Changed 9 years ago by
- 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: ↓ 8 Changed 9 years ago by
comment:8 in reply to: ↑ 7 Changed 9 years ago by
- 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
- Description modified (diff)
- Resolution set to duplicate
- Status changed from positive_review to closed
Patch that adds components of units.si_prefixes as properties on the UnitExpression? class