Merge Raw vis into Scalar vis#1770
Conversation
|
|
||
| {isImage ? ( | ||
| <RawImageVis | ||
| <BinaryImageVis |
There was a problem hiding this comment.
I renamed this component, since the term "Raw" no longer makes much sense.
axelboc
force-pushed
the
array-vis
branch
2 times, most recently
from
f224ad4 to
193b553
Compare
axelboc
force-pushed
the
array-vis
branch
8 times, most recently
from
7260a8c to
ecff171
Compare
| assertScalarShape(entity); | ||
| assertPrintableType(entity); |
There was a problem hiding this comment.
ScalarVisContainer now accepts anything with a non-null shape.
| const scalarUint = dataset('uint', scalarShape(), intType(false)); | ||
| const scalarBigInt = dataset('bigint', scalarShape(), intType(true, 64)); | ||
| const scalarFloat = dataset('float', scalarShape(), floatType()); | ||
| const scalarStr = dataset('float', scalarShape(), strType()); | ||
| const scalarBool = dataset('bool', scalarShape(), boolType(intType(true, 8))); |
There was a problem hiding this comment.
Testing CORE_VIS.Scalar with all these is now overkill since it doesn't care about the dtype any more.
| onChange={setDimMapping} | ||
| /> | ||
| )} | ||
| <VisBoundary resetKey={dimMapping} isSlice={dims.length > 0}> |
There was a problem hiding this comment.
The code comes from RawVisContainer with one subtle fix here caught by our Scalar vis tests: isSlice is now set dynamically to ensure that the correct loading message is shown ("Loading data" instead of "Loading slice" when fetching the value of a scalar dataset)
loichuder
left a comment
loichuder
left a comment
There was a problem hiding this comment.
Implementation looks good but it feels very wrong to call Scalar, something that actually support any shape, including images.
Perhaps we should have merged Scalar vis into Raw vis ? 😂
|
Yeah, I had the same hesitation... I was initially going to keep only the Raw vis but ended up changing my mind. I reminded myself that:
I also didn't like the term Raw for visualizing simple scalars like numbers, strings, booleans, etc. |
|
What about Primitive? |
2 participants
Finally getting rid of some code 😂
The diff is not tooo bad, but I've actually done the refactoring in three commits:
RawVisso it just accepts astringvalue likeScalarVis.RawVisContainerhandle printable scalar datasets and removedScalarVisContainerandScalarViscompletely. The core change here was the merger of the scalar vis formatter into the raw vis formatter.You'll note that none of the screenshots change. The only visible changes for users are that:
ScalarToolbar, previouslyRawToolbar) with the "Fit image" control and the ability to export to JSON.I'm planning to clean-up the mock file and the tests a fair bit (
raw,raw_large, etc.) but didn't want to clutter this PR, which is already confusing because of all the files that are moved and renamed.