More intuitive display of negative time deltas (#8274)

[benvdh]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

This pull request changes the way negative time delta's are displayed. The default python display of a negative timedelta is rather unintuitive for a user of a dashboard. For example, -6 minutes is displayed as '-1 day, 23:54:00. This basically requires users to make time calculations in their heads to interpret the number correctly. This pull request changes the formatting to '-0:06:00' at the code level and -0 days 00:06:00 at the superset UI level to make it easier for users to interpret the time delta. More details on the issue and why this solution was chosen can be found in #8274 .

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

The data in postgres looks like this:

Before this pull request the data looks like this in superset:

Afterwards it looks like this in superset:

TEST PLAN

• The unit test for base_json_conv() has been enhanced to handle timedelta's as well
• A new function has been added in superset.utils.core named timedelta_f which is called in base_json_conv() (same module). A new test has been added to test this formatting function.

ADDITIONAL INFORMATION

• Has associated issue: Intuitive display of negative timedeltas #8274
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

[codecov-io]

Codecov Report

Merging #8276 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #8276      +/-   ##
==========================================
+ Coverage   67.72%   67.73%   +0.01%     
==========================================
  Files         451      451              
  Lines       22674    22678       +4     
  Branches     2370     2370              
==========================================
+ Hits        15355    15360       +5     
+ Misses       7182     7181       -1     
  Partials      137      137

Impacted Files	Coverage Δ
superset/utils/core.py	88.57% <100%> (+0.24%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7090725...7d1eb56. Read the comment docs.

[villebro]

Left some comments. Also, do check out #8136 which might be relevant for you. I don't think it properly works in all charts yet, but if this is something that more people find useful I could probably work on improving support for durations.

[benvdh]

Left some comments. Also, do check out #8136 which might be relevant for you. I don't think it properly works in all charts yet, but if this is something that more people find useful I could probably work on improving support for durations.

Thanks for the quick review and helpful comments 👍 . They definitely improved the code. #8136 looks quite interesting and useful to my use case (described in the linked issue). Will play around with it when I find some time.

[villebro]

Sorry for not catching this last pass, but having a more verbose name might be more clear. Other than that LGTM 👍

[benvdh]

@villebro I have renamed the function, but now suddenly the CI pipeline is failing on the cypress-sqllab test. This seems to be a known issue being addressed in open pull request #8215. Should we wait until this is fixed?

[villebro]

This is a known unrelated issue; I kicked off CI again.

[benvdh]

This is a known unrelated issue; I kicked off CI again.

@villebro Thanks! Unfortunately, now it's the cypress-dashboard test (which passed in the previous built) with a similar unrelated timeOut issue:

 1) Dashboard top-level controls should allow chart level refresh:

     CypressError: Timed out retrying: cy.wait() timed out waiting 5000ms for the 1st request to the route: 'postJson_8_force'. No request ever occurred.

Are there anymore changes I need to make before this pull request can be approved?

[villebro]

LGTM. I'm not an expert on negative time durations, but this might be something that should be brought to the attention of the python core devs.