devansh173 opened a new pull request, #44644:
URL: https://github.com/apache/superset/pull/44644

   ### SUMMARY
   Text reports format numbers on the server with 
`superset/utils/number_format.py`, a port of the frontend's d3-format. 
d3-format hides the sign of a negative value that rounds to zero at the chosen 
precision (unless the `+` sign mode is used), but `apply_sign` decided the sign 
from the raw value. As a result, report tables showed `-0.00`, `(0.00)`, 
`-$0.00` or `-0` where the chart in the browser shows `0.00`, `0.00`, `$0.00` 
or `0`.
   
   This adds the same rule d3 has in `src/locale.js`:
   
   ```js
   // If a negative value rounds to zero after formatting, and no explicit 
positive sign is requested, hide the sign.
   if (valueNegative && +value === 0 && sign !== "+") valueNegative = false;
   ```
   
   The sign is taken from `math.copysign` so `-0.0` is handled like d3's `1 / 
value < 0` check (`+,.2f` of `-0.0` is `-0.00` in both).
   
   | format | value | before | after (= d3-format) |
   |---|---|---|---|
   | `,.2f` | -0.001 | `-0.00` | `0.00` |
   | `(,.2f` | -0.001 | `(0.00)` | `0.00` |
   | `$,.2f` | -0.004 | `-$0.00` | `$0.00` |
   | `.1%` | -0.0001 | `-0.0%` | `0.0%` |
   | `,d` | -0.4 | `-0` | `0` |
   | `+,.2f` | -0.001 | `-0.00` | `-0.00` |
   
   The expected values were generated with d3-format 3.1.2 using Superset's 
locale (`minus: "-"`).
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A
   
   ### TESTING INSTRUCTIONS
   Run the unit tests:
   
   ```
   pytest tests/unit_tests/utils/number_format_test.py
   ```
   
   Or check manually:
   
   ```python
   from superset.utils.number_format import format_number_with_config
   format_number_with_config(",.2f", None, -0.001)   # before: "-0.00", after: 
"0.00"
   format_number_with_config("(,.2f", None, -0.001)  # before: "(0.00)", after: 
"0.00"
   format_number_with_config(",.2f", None, -1.5)     # "-1.50"
   ```
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: Fixes #44642
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to