-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/add export and import data #183
base: develop
Are you sure you want to change the base?
Changes from 7 commits
d5f95c7
782a7bb
e0cd8b9
e8d1d6a
62073aa
5086f4b
012236b
f2ddbfd
5cc6831
9fb1e4b
77ca8f4
41007d5
36db7da
329ef8e
f4163bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
//e.g. 1.2.0 is described as [1, 2, 0] | ||
export type SemanticVersion = [number, number, number] | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { SemanticVersion } from '@/@types/types' | ||
import { configDataSchema } from './configDataSchema' | ||
import { PlotDataSchema } from './plotDataSchema' | ||
|
||
export interface allDataSchema { | ||
appVersion: SemanticVersion | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a type, it's a semantic version, and when actually used, it's the app version. This usage looks good. |
||
plotImageBase64: string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added plot image as base 64 string. |
||
plotData: PlotDataSchema | ||
configData: configDataSchema | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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') 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 |
||
schemaVersion: SemanticVersion | ||
figure: { | ||
scale: number | ||
} | ||
axisSets: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it might be better to use array type annotations here for clarity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in: 77ca8f4 |
||
id: number | ||
pointMode: 0 | 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since it’s a magic number, it would be good to address it later There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in: 5cc6831 |
||
isVisible: boolean | ||
shouldConsiderGraphTilt: boolean | ||
}[] | ||
interpolator: { | ||
isActive: boolean | ||
interval: number | ||
} | ||
magnifier: { | ||
magnificationTimes: number | ||
} | ||
extractor: { | ||
strategy: 'Symbol Extract' | 'Line Extract' | ||
colorDiffThresholdPercent: number | ||
symbolExtraction: { | ||
minDiameterPx: number | ||
maxDiamiterPx: number | ||
} | ||
lineExtraction: { | ||
deltaXPx: number | ||
deltaYPx: number | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
//NOTE: Includes the actual data collected from a figure: axis values, axis scale, data point values and so | ||
//NOTE: Do not include any data which depends on StarryDigitizer app(e.g. setting axis by 2 points or 4 points etc...) | ||
|
||
import { SemanticVersion } from '@/@types/types' | ||
//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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that configDataSchema and PlotDataSchema are not consistently using camelCase and PascalCase. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in: 9fb1e4b |
||
schemaVersion: SemanticVersion | ||
axisSets: Array<{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Used Array<{}> instead of {}[] |
||
id: number | ||
name: string | ||
x1: { | ||
value: number | ||
coord: Coord | ||
} | ||
x2: { | ||
value: number | ||
coord: Coord | ||
} | ||
y1: { | ||
value: number | ||
coord: Coord | ||
} | ||
y2: { | ||
value: number | ||
coord: Coord | ||
} | ||
xScale: 'linear' | 'log' //TODO: create type AxisScale: 'linear' | 'log' | ||
yScale: 'linear' | 'log' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated overall schema of axis set to be more simple and following the original data structure of StarryDigitizer. |
||
}> | ||
datasets: Array<{ | ||
id: number | ||
name: string | ||
axisSetId: number | ||
points: Array<{ | ||
value: number | ||
coord: Coord | ||
}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the pixelDataSchema and integrated it into the plotDataSchema: now each axis and point has the combination of coordinates(pixel) and an actual value |
||
}> | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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...