Replace fixed-point with exact notation in Matrix and Compound vis #1753
+24
−44
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
axelboc
commented
Feb 4, 2025
Comment on lines
+75
to
+76
| getFormatter(type.realType, notation), | ||
| getFormatter(type.imagType, notation), |
There was a problem hiding this comment.
I recurse to inherit the formatting of integers/floats.
| @@ -216,7 +216,7 @@ export interface UnknownType { | |||
| export type ScalarValue<T extends DType = DType> = T extends NumericLikeType | |||
| ? | |||
| | number | |||
| | (T extends NumericType ? bigint : never) // let providers return bigints | |||
| | (T extends IntegerType ? bigint : never) // let providers return bigints | |||
There was a problem hiding this comment.
Unrelated. This is more precise but has no impact on the codebase since we don't have visualizations that accept only integers or only floats.
Comment on lines
-75
to
-81
| if (imag === 0 && !full) { | ||
| return formatVal(real); | ||
| } | ||
|
|
||
| if (real === 0 && !full) { | ||
| return `${formatVal(imag)} i`; | ||
| } |
There was a problem hiding this comment.
I'm removing the feature of showing only the real part or only the imaginary part if the other part is equal to 0.
|
/approve |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed in #1646 (comment). Keeping the issue open until I address the formatting of numbers in the visualizations' tooltips.
This should also address the user need of #1742 in a more usable way than the solution proposed of adding a control to the toolbar to increase the fixed point precision.
While I'm at it, I bring the formatting of complex numbers in line with the formatting of other numbers. We were so far using formatters with less precision to save space in the Matrix vis, but I think consistency should prime.
Here is a more precise list of user-facing changes:
< Scalar >Display complex numbers with exact precision< Scalar >Always show both real and imaginary parts of complex numbers to avoid confusing complex numbers without imaginary parts with real numbers — this applies also when inspecting values of complex attributes in the metadata viewer< Matrix >Replace Fixed-point notation with Exact notation to allow inspecting and copying exact values< Matrix >Inherit integer and/or float formatting for complex numbers in Auto and Scientific notations (depending on dtype of real and imaginary parts) — concretely, complex numbers are now displayed with more precision regardless of the notation.