Skip to content
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

ui: fix mixed units in tooltips #40970

Merged
merged 1 commit into from
Oct 29, 2019

Conversation

vladlos
Copy link
Contributor

@vladlos vladlos commented Sep 23, 2019

We already calculate biggest scale for chart axis label, so I take that and cast values in chart tooltips to fit chart scale. So tooltips will show consistent units, matching the biggest unit type in data set and axis label. And also has 2 numbers after dot for better readability. This was done for duration and bytes units.

Resolves: #35467

Release note (ui): consistent units in chart tooltips

result:
Screen Shot 2019-09-23 at 6 11 50 PM

@vladlos vladlos requested a review from a team September 23, 2019 10:14
@CLAassistant
Copy link

CLAassistant commented Sep 23, 2019

CLA assistant check
All committers have signed the CLA.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nathanstilwell nathanstilwell self-assigned this Sep 23, 2019
@nathanstilwell nathanstilwell added the first-pr Use to mark the first PR sent by a contributor / team member. Reviewers should be mindful of this. label Sep 23, 2019
Copy link
Contributor

@nathanstilwell nathanstilwell left a comment

Choose a reason for hiding this comment

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

The code looks good, but I would like to see some unit tests for the format functions. This made me realize that none of the other format utils are tested, but I suppose we have to start somewhere.

@nathanstilwell
Copy link
Contributor

@elkmaster You may disregard the this build failure for now. When I release branch is cut the justification requirement will be lifted.

@vladlos vladlos force-pushed the fix-chart-tooltip-mixed-units branch from 3d1e666 to ec5d109 Compare September 24, 2019 14:20
@vladlos
Copy link
Contributor Author

vladlos commented Sep 24, 2019

@nathanstilwell added unit tests for my code

We already calculate biggest scale for chart axis label, so I take that and cast values in chart tooltips to fit chart scale. So tooltips will show consistent units, matching the biggest unit type in data set and axis label. And also has 2 numbers after dot for better readability. This was done for duration and bytes units.

Resolves: cockroachdb#35467

Release note (ui): consistent units in chart tooltips
@nathanstilwell
Copy link
Contributor

bors r+

craig bot pushed a commit that referenced this pull request Oct 29, 2019
40970: ui: fix mixed units in tooltips r=nathanstilwell a=elkmaster

We already calculate biggest scale for chart axis label, so I take that and cast values in chart tooltips to fit chart scale. So tooltips will show consistent units, matching the biggest unit type in data set and axis label. And also has 2 numbers after dot for better readability. This was done for duration and bytes units.

Resolves: #35467

Release note (ui): consistent units in chart tooltips

result:
![Screen Shot 2019-09-23 at 6 11 50 PM](https://user-images.githubusercontent.com/12850886/65438206-c2868080-de2d-11e9-9058-be07dee2c86a.png)



41724: coldata: use flat bytes implementation instead of [][]byte r=yuzefovich a=yuzefovich

**coldata: use flat bytes implementation instead of [][]byte**

This commit reverts the revert of using []byte + offsets implementation
of two-dimensional byte slice. The original revert was done due to
several issues which will be addressed in the follow-up commits.

**coldata: fix a couple of issues with flat bytes implementation**

This commit fixes the following:
- Slice method no longer modifies the receiver in-place and returns
  a shallow copy instead (as regular slices do)
- in CopySlice, srcEndIdx has been updated to be "trimmed" so that
  offsets and lengths are not overwritten past the copied values.

**coldata: remove lengths slice from flat bytes implementation**

Previously, we would use offsets and lengths to store the
information about each of the []byte in the flat bytes.
However, that information was quite redundant since
b.lengths[i] = b.offsets[i+1] - b.offsets[i], and this commit
removes that redundancy.

Also, this commit fixes a problem with how NULLs are treated.
Previously, it was possible for offsets of flat bytes to get into
an "inconsistent" state - we assume that the offsets are
non-decreasing, but when a NULL value is set on the colexec.Vec,
then Bytes will not be notified of this. As a result, there were
"gaps" of unset zero values in offsets. Now we're actively
backfilling the offsets to maintain the invariant.

Fixes: #40244.

**colexec: add invariants checker during logic tests**

This commit adds an ability to plan vectorized invariants
checkers while running the logic tests (in all configs except
for when the vectorized engine is disabled). The only invariants
that are currently being checked are that the batches are of correct
width and that all byte slices (up to the length of the batch) can be
retrieved from the flat bytes implementation.

Release note: None

41926: sql: don't look up FK metadata when not checking FKs r=RaduBerinde a=RaduBerinde

The `fkTables` argument to `MakeInserter` is only used when we are
checking FKs; don't generate it when skipping the old path.

Release note: None

41975: add otan to AUTHORS file r=otan a=otan



Co-authored-by: Vlad <carrott9@gmail.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented Oct 29, 2019

Build succeeded

@craig craig bot merged commit 1521b62 into cockroachdb:master Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-pr Use to mark the first PR sent by a contributor / team member. Reviewers should be mindful of this.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ui: Timeseries legends mix and match unit sizes
4 participants