Feature/add export and import data #183
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
| import { PlotDataScheme } from './plotDataScheme' | ||
|
|
||
| export interface allDataScheme { | ||
| appVersion: string |
There was a problem hiding this comment.
Not sure if we need app version in the json data, but currently each scheme has its own scheme version and all-data scheme has an app version.
Another option is to have only one global scheme version on the allDataScheme, and not to have app version or have both
There was a problem hiding this comment.
If it will be a single JSON, then having just one would be fine. However, if multiple JSONs are generated upon output, it might be better to have each separately. I assume it's the former, though
There was a problem hiding this comment.
ah, this first comment looks outdated 😓
There was a problem hiding this comment.
AllDataSchema will be one single JSON, but each of PlotDataSchema and ConfigDataSchema might be imported in other export formats.
So for now I think it would be better to have schema versions for each🤔
| @@ -0,0 +1,2 @@ | |||
| //e.g. 1.2.0 is described as [1, 2, 0] | |||
| export type SemanticVersion = [number, number, number] | |||
There was a problem hiding this comment.
Added new type 'SemanticVersion' to describe versions as an array of numbers(referred to WPD json
schema)
SemanticVersionConverter(string <-> array) will be implemented soon, mauybe...
| points: Array<{ | ||
| value: number | ||
| coord: Coord | ||
| }> |
There was a problem hiding this comment.
Removed the pixelDataSchema and integrated it into the plotDataSchema: now each axis and point has the combination of coordinates(pixel) and an actual value
|
|
||
| export interface PlotDataSchema { | ||
| schemaVersion: SemanticVersion | ||
| axisSets: Array<{ |
There was a problem hiding this comment.
Used Array<{}> instead of {}[]
I think Array<{}> is better to clearly know it is array
| coord: Coord | ||
| } | ||
| xScale: 'linear' | 'log' //TODO: create type AxisScale: 'linear' | 'log' | ||
| yScale: 'linear' | 'log' |
There was a problem hiding this comment.
Updated overall schema of axis set to be more simple and following the original data structure of StarryDigitizer.
|
|
||
| export interface allDataSchema { | ||
| appVersion: SemanticVersion | ||
| plotImageBase64: string |
There was a problem hiding this comment.
Added plot image as base 64 string.
The reason why it is not in the plotDataSchema is that we may want to export plot data without image data.
| //TODO: move definition of type Coord to more global directoly (e.g. @types) | ||
| import { Coord } from '@/domain/models/dataset/datasetInterface' | ||
|
|
||
| export interface PlotDataSchema { |
There was a problem hiding this comment.
It seems that configDataSchema and PlotDataSchema are not consistently using camelCase and PascalCase.
| import { SemanticVersion } from '@/@types/types' | ||
|
|
||
| //NOTE: Add any information about StarryDigitizer configurations which is necassary to reconstruct the application states here. | ||
| export interface configDataSchema { |
There was a problem hiding this comment.
I believe that the interfaces used for external integration are named 〇〇DataSchema, and those used internally are named 〇〇interface, but I think people who are new to the code might not understand this distinction. It might be better to either change the naming or leave a note somewhere to clarify the usage.
There was a problem hiding this comment.
Thank you. I will leave a comment about that.
BTW, TS Deep Dive says that we should avoid prefix 'I' on the interface names (which I guess is similar to put suffix 'Interface')
This is one of my current interests about TS and thinking how to avoid conflict between interface names and class name
https://typescript-jp.gitbook.io/deep-dive/styleguide#%E3%82%A4%E3%83%B3%E3%82%BF%E3%83%BC%E3%83%95%E3%82%A7%E3%83%BC%E3%82%B9
(Sadly, the anchor link is not working...)
| figure: { | ||
| scale: number | ||
| } | ||
| axisSets: { |
There was a problem hiding this comment.
it might be better to use array type annotations here for clarity.
| } | ||
| axisSets: { | ||
| id: number | ||
| pointMode: 0 | 1 |
There was a problem hiding this comment.
Since it’s a magic number, it would be good to address it later
| import { PlotDataSchema } from './plotDataSchema' | ||
|
|
||
| export interface allDataSchema { | ||
| appVersion: SemanticVersion |
There was a problem hiding this comment.
As a type, it's a semantic version, and when actually used, it's the app version. This usage looks good.
# Conflicts: # src/@types/types.d.ts
# Conflicts: # src/@types/types.d.ts
No description provided.