-
Notifications
You must be signed in to change notification settings - Fork 121
Fix raw formatting for big numbers in parameter export and templating #12535
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12535 +/- ##
==========================================
- Coverage 90.58% 90.55% -0.03%
==========================================
Files 435 435
Lines 29931 29943 +12
==========================================
+ Hits 27112 27116 +4
- Misses 2819 2827 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
33ebc62 to
49d268f
Compare
| if isinstance(value, int): | ||
| value_str = str(value) | ||
| elif isinstance(value, float): | ||
| value_str = f"{value:.6g}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want this at .6g formatting? What will happen if the value is 1234.567464646?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that and it might get flaky without a precision; ie. .6g means we get always a correct rounding of the number. There are many corner cases though. Extra large and extra small numbers - not sure how realistic they are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure this will fix the issue, as the issue is the cornercase when the number is very large. I did some querying and one number submitted by the user was 257382775096236763020404641646935773111
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but that is an integer (very likely from the design matrix), which should get a correct handling as integer.
|
Can we add the test from #12480? |
|
We need an extra pair of eyes on this one: @oyvindeide |
|
We do not want to round as far down as I would expect Ert to let me work with double precision floats. |
We need to round it for some decimals though; ie. tests, snapshots, etc |
Rounding for that purpose should be put in the test code.
|
will not work: >>> e = 0.000000000000001
>>> repr(e)
'1e-15' |
|
Follow up issue with floating precision: #12581 |
oyvindeide
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one minor comment.
| "REAL": list(range(num_realizations)), | ||
| "a": a_values, | ||
| "category": 5 * ["cat1"] + 5 * ["cat2"], | ||
| "big_numbers": [1e10 + i for i in range(num_realizations)], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps rename this to big_integer for clarity?
This makes sure that we treat integers in raw format, while floats will remain with the existing scientific notation.
|
Successfully created backport PR for |
Issue
Resolves #12427
Approach
Make sure that the formatting stays raw for integers.
Floats will use notation with
:gto keep the current state.(Screenshot of new behavior in GUI if applicable)
git rebase -i main --exec 'just rapid-tests')When applicable