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

Update brushable histogram x-tick number format #1363

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

mweiden
Copy link
Contributor

@mweiden mweiden commented Apr 6, 2020

Fixes #1349

Sub-issues

Repeating tick mark issue

The issue was that significant digits that differentiated consecutive tick marks were cropped in number string formatting.

Strategy to fix number formatting: increase the number of significant digits shown in scientific notation number formatting to 2 (format .2e) instead of 1 (format .1s).

For more information see https://github.com/d3/d3-format

Note that does not fully fix the issue described in #1349, but rather makes the formatting issue far less likely. It is still possible for this to occur if the difference between two ticks in axes happens in the a significant digit cropped by the scientific notation format.

milli notation (123m) is not intuitive

Users reported that using an m to denote a number (trueValue = 0.001 * displayNumber) was not intuitive. This PR changes the behavior of the formatting logic to use unabbreviated numbers when the domain of a variable is -10000 < min < max < 10000 and use scientific notation otherwise. We let d3 determine xticks for decimals since it generally choses values without many significant digits.

Results

The following are images show improvements over the example problems in #1349:

Screen Shot 2020-04-06 at 5 44 24 PM

litvinukova20_restricted.processed.h5ad

Screen Shot 2020-04-06 at 5 42 22 PM

lukassen20_airway_orig.processed.h5ad

@mweiden mweiden self-assigned this Apr 6, 2020
@mweiden mweiden requested review from seve and bkmartinjr April 6, 2020 22:26
@mweiden mweiden force-pushed the mweiden/1349-fix-hist-x-ticks branch from afc58c2 to abe4b6f Compare April 6, 2020 22:28
@codecov-io
Copy link

codecov-io commented Apr 6, 2020

Codecov Report

Merging #1363 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1363   +/-   ##
=======================================
  Coverage   62.22%   62.22%           
=======================================
  Files          67       67           
  Lines        5152     5152           
  Branches      377      377           
=======================================
  Hits         3206     3206           
  Misses       1854     1854           
  Partials       92       92           
Flag Coverage Δ
#backend 52.16% <ø> (ø)
#frontend 75.26% <ø> (ø)
#javascript 75.26% <ø> (ø)
#python 52.16% <ø> (ø)
#smokeTest ?
#smokeTestAnnotations 100.00% <ø> (ø)
#unitTest 62.22% <ø> (ø)

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 4efabf5...588f59e. Read the comment docs.

Copy link
Contributor

@bkmartinjr bkmartinjr left a comment

Choose a reason for hiding this comment

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

Technically the changes look fine. However, we need to decide if this is the right UI change.

@mweiden mweiden force-pushed the mweiden/1349-fix-hist-x-ticks branch from abe4b6f to 588f59e Compare April 7, 2020 02:48
Fixes #1349

For more information see https://github.com/d3/d3-format

Note that does not _fully_ fix the issue described in #1349, but rather
makes the formatting issue far less likely. It is _still_ possible for
this to occur if the difference between two ticks in axes happes in the
a significant digit cropped by the scientific notation format
@mweiden mweiden force-pushed the mweiden/1349-fix-hist-x-ticks branch from 588f59e to 35778eb Compare April 7, 2020 04:08
@mweiden mweiden changed the title Hotfix for brushable histogram x-tick formatting issues Update brushable histogram x-tick number format Apr 7, 2020
@mweiden
Copy link
Contributor Author

mweiden commented Apr 8, 2020

Got positive feedback on this from users. Merging this. Will make further adjustments in the lead up to 0.16.0, if necessary.

@mweiden mweiden merged commit 1f94c71 into master Apr 8, 2020
@mweiden mweiden deleted the mweiden/1349-fix-hist-x-ticks branch April 8, 2020 16:45
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.

Incorrect x axis tick label for continuous metadata histogram
3 participants