Skip to content

Conversation

maurizi
Copy link
Contributor

@maurizi maurizi commented Aug 31, 2017

Connects to #2733

@maurizi maurizi requested a review from jwalgran August 31, 2017 18:54
@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 83.852% when pulling 20f5930 on maurizi:no-scientific-notation into 52a89f6 on OpenTreeMap:develop.

@jwalgran
Copy link
Contributor

The tag is working well for the built in fields but I added a custom Decimal field and the view mode value is getting scientifically notated.

Looks good in the form

screen shot 2017-08-31 at 2 42 47 pm

Notated after save

screen shot 2017-08-31 at 2 42 06 pm

Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom decimal field values in the view mode are getting scientifically notated.

@maurizi maurizi force-pushed the no-scientific-notation branch from 20f5930 to 537c391 Compare September 5, 2017 21:53
@maurizi
Copy link
Contributor Author

maurizi commented Sep 5, 2017

Custom decimal field values in the view mode are getting scientifically notated.

Fixed in (rebased) commit 537c391. I also fixed formatting for numbers in the edits sidebar, and made sure we don't show trailing zeros

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.006%) to 83.927% when pulling 537c391 on maurizi:no-scientific-notation into e558d2a on OpenTreeMap:develop.

Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest updates get rid of scientific notation everywhere, but we have some trailing periods to deal with.

def num_format(num):
if isinstance(num, float):
# Allow for up to 10 digits of precision, but strip trailing 0s
return '{0:.10f}'.format(num).rstrip('0')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For any "whole number" value like 123.0 this rstrip is leaving a trailing .

screen shot 2017-09-06 at 9 55 21 am

screen shot 2017-09-06 at 10 00 55 am

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7bdee5f

Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need one more tweak to the zero handing.

def num_format(num):
if isinstance(num, float):
# Allow for up to 10 digits of precision, but strip trailing '0' or '.'
return '{0:.10f}'.format(num).rstrip('0.')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this rstrip call will remove too many characters when the whole number portion of the decimal ends in 0.

In [1]: '120.0'.rstrip('0.')
Out[1]: '12'

Chaining rstrips looks like it will give us what we are looking for

In [2]: '120.0'.rstrip('0').rstrip('.')
Out[2]: '120'

In [3]: '120.01'.rstrip('0').rstrip('.')
Out[3]: '120.01'

In [4]: '120.010'.rstrip('0').rstrip('.')
Out[4]: '120.01'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh... sorry for all the back and forth. Fixed in 149dfea

@maurizi maurizi force-pushed the no-scientific-notation branch from 7bdee5f to 149dfea Compare September 6, 2017 19:12
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 83.92% when pulling 7bdee5f on maurizi:no-scientific-notation into e558d2a on OpenTreeMap:develop.

@maurizi maurizi merged commit 2de3819 into OpenTreeMap:develop Sep 7, 2017
@maurizi maurizi deleted the no-scientific-notation branch September 7, 2017 00:24
@maurizi maurizi removed the in review label Sep 7, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.006%) to 83.904% when pulling 149dfea on maurizi:no-scientific-notation into 7b3f08b on OpenTreeMap:develop.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants