Skip to content

Commit

Permalink
Merge pull request #1694
Browse files Browse the repository at this point in the history 
* * In this branch we're marking instances of TODO_V2_IMPORT with indic…

* * Explicitly classified ~24 of the TODO_V2_IMPORT comments. ~49 remai…
  • Loading branch information
@bfinzer
bfinzer authored Dec 27, 2024
1 parent 082724f commit 72b1c32
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 34 deletions.
10 changes: 5 additions & 5 deletions v3/src/components/graph/adornments/v2-adornment-importer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,11 @@ interface IInstanceKeysForAdornmentsProps {
}

const univariateMeasureInstances = (adornment: ICodapV2UnivariateAdornment, instanceKeys?: string[]) => {
// TODO_V2_IMPORT: most documents with an equationCoordsArray have multiple items in the array
// TODO_V2_IMPORT: [Story **#188695360**] most documents with an equationCoordsArray have multiple items in the array
// we are only looking at the first item here
const equationCoordsV2 = adornment.equationCoordsArray?.[0]

// TODO_V2_IMPORT: 93 files in cfm-shared have equationCoordsArray with values with
// TODO_V2_IMPORT: [Story: **#188695677**] 93 files in cfm-shared have equationCoordsArray with values with
// proportionCenterX and proportionCenterY instead of proportionX and proportionY.
// For now we are just skipping those and treating them as undefined
// example doc: cfm-shared/1caDhoHFlpuNQgSfOdhh/file.json
Expand Down Expand Up @@ -223,7 +223,7 @@ export const v2AdornmentImporter = ({data, plotModels, attributeDescriptions, yA
const lines: Record<string, IMovableLineInstanceSnapshot> = {}
instanceKeys?.forEach((key: string) => {
const lineInstance = {
// TODO_V2_IMPORT: equationCoords are not handled correctly, the model stores x and y
// TODO_V2_IMPORT: [Story: **#188695677**] equationCoords are not handled correctly, the model stores x and y
// but the loaded equationCoords have proportionCenterX and proportionCenterY
equationCoords: equationCoords ?? undefined, // The V2 default is null, but we want undefined
intercept: movableLineAdornment.intercept,
Expand All @@ -249,7 +249,7 @@ export const v2AdornmentImporter = ({data, plotModels, attributeDescriptions, yA
const lsrlInstances: ILSRLInstanceSnapshot[] = []
lsrlAdornment.lsrls?.forEach((lsrl) => {
const lsrlInstance = {
// TODO_V2_IMPORT: equationCoords are not handled correctly, the model stores x and y
// TODO_V2_IMPORT: [Story: **#188695677**] equationCoords are not handled correctly, the model stores x and y
// but the loaded equationCoords have proportionCenterX and proportionCenterY
equationCoords: lsrl.equationCoords ?? undefined // The V2 default is null, but we want undefined
}
Expand Down Expand Up @@ -377,7 +377,7 @@ export const v2AdornmentImporter = ({data, plotModels, attributeDescriptions, yA
plotValues.push(value.value)
})

// TODO_V2_IMPORT: both valueModels and values might have `isVisible: false`
// [Story: #188699857] TODO_V2_IMPORT: both valueModels and values might have `isVisible: false`
// we are currently just ignoring that
values[key] = plotValues
})
Expand Down
5 changes: 3 additions & 2 deletions v3/src/components/graph/v2-graph-importer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ export function v2GraphImporter({v2Component, v2Document, sharedModelManager, in
pointColor, strokeColor, pointSizeMultiplier,
strokeSameAsFill, isTransparent,
plotBackgroundImageLockInfo,
/* TODO_V2_IMPORT: The following are present in the componentStorage but not used in the V3 content model (yet):
/* TODO_V2_IMPORT: [Story: #188694812]
The following are present in the componentStorage but not used in the V3 content model (yet):
displayOnlySelected, legendRole, legendAttributeType, numberOfLegendQuantiles,
legendQuantilesAreLocked, plotBackgroundImage, transparency, strokeTransparency,
plotBackgroundOpacity,
Expand Down Expand Up @@ -132,7 +133,7 @@ export function v2GraphImporter({v2Component, v2Document, sharedModelManager, in
axes[v3Place] = {place: v3Place, type: "categorical"}
break
case "DG.CellLinearAxisModel":
// TODO_V2_IMPORT when lowerBound or upperBound are undefined or null this is
// TODO_V2_IMPORT [Story:#188701144] when lowerBound or upperBound are undefined or null this is
// not handled correctly. It likely will cause an MST exception and failure to load.
// There are 966 instances of `xUpperBound: null` in cfm-shared
axes[v3Place] = {place: v3Place, type: "numeric", min: lowerBound as any, max: upperBound as any}
Expand Down
6 changes: 3 additions & 3 deletions v3/src/components/map/v2-map-importer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function v2MapImporter({v2Component, v2Document, insertTile}: V2TileImpor
const {
pointColor, strokeColor, pointSizeMultiplier,
grid, pointsShouldBeVisible, connectingLines
/* TODO_V2_IMPORT: Present in v2 layer model but not yet used in V3 layer model:
/* TODO_V2_IMPORT: [Story: #188694812] Present in v2 layer model but not yet used in V3 layer model:
transparency, strokeTransparency
*/
} = v2LayerModel
Expand All @@ -81,7 +81,7 @@ export function v2MapImporter({v2Component, v2Document, insertTile}: V2TileImpor
dataset: data?.dataSet.id,
metadata: metadata?.id,
_attributeDescriptions,
// TODO_V2_IMPORT hiddenCases are not imported
// TODO_V2_IMPORT [Story: #188694826] hiddenCases are not imported
// the array in a "modern" v2 document coming from `mapModelStorage.layerModels[]._links_.hiddenCases`
// looks like { type: "DG.Case", id: number }
// The MST type expects an array of strings.
Expand All @@ -106,7 +106,7 @@ export function v2MapImporter({v2Component, v2Document, insertTile}: V2TileImpor
else if (isV2MapPolygonLayerStorage(v2LayerModel)) {
const {
areaColor, areaStrokeColor,
/* TODO_V2_IMPORT: Present in v2 layer model but not yet used in V3 layer model:
/* TODO_V2_IMPORT: [Story: #188694812] Present in v2 layer model but not yet used in V3 layer model:
areaTransparency, areaStrokeTransparency
*/
} = v2LayerModel
Expand Down
3 changes: 2 additions & 1 deletion v3/src/v2/codap-v2-document.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ export class CodapV2Document {

registerContexts(contexts?: CodapV2Context[]) {
contexts?.forEach(context => {
// TODO_V2_IMPORT: external contexts are not imported
// TODO_V2_IMPORT_IGNORE: external contexts are not imported
// There are 75 cases of external contexts in cfm-shared
// This refers to an obsolescent form of document storage no longer used at all. Ignore!
if (isV2ExternalContext(context)) return
const { guid, type = "DG.DataContext", document, name = "", title, collections = [] } = context
if (document && this.guidMap.get(document)?.type !== "DG.Document") {
Expand Down
54 changes: 31 additions & 23 deletions v3/src/v2/codap-v2-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ export interface ICodapV2Attribute {
deletedFormula?: string
precision?: number | string | null
unit?: string | null
// TODO_V2_IMPORT decimals is not imported
// TODO_V2_IMPORT_CARRY_OVER [Story:#188701222] decimals is not imported
// Defined as `precision` in v3
// it occurs 604 times in 36 files in cfm-shared
// in 33 cases the value is "2"
// the rest of the cases the value is "0"
Expand Down Expand Up @@ -58,26 +59,27 @@ export interface ICodapV2Case {
export interface ICodapV2Collection {
attrs: ICodapV2Attribute[]
cases: ICodapV2Case[]
// TODO_V2_IMPORT: caseName is not imported
// TODO_V2_IMPORT_EXTRACT: caseName is not imported
// There are 2,500 cases where this has a value in cfm-shared
caseName?: string | null
// TODO_V2_IMPORT: childAttrName is not imported
// TODO_V2_IMPORT_EXTRACT: childAttrName is not imported
// There are 8,592 cases where this has a value in cfm-shared
childAttrName?: string | null
cid?: string
// TODO_V2_IMPORT: collapseChildren is not imported
// TODO_V2_IMPORT_EXTRACT: collapseChildren is not imported
// There are 250 cases where this is true in cfm-shared
collapseChildren?: boolean | null
// TODO_V2_IMPORT: defaults does not seem to be imported
// TODO_V2_IMPORT_EXTRACT: defaults does not seem to be imported
// There are 825 cases where it is defined in cfm-shared
defaults?: {
xAttr?: string
yAttr?: string
}
guid: number
id?: number
// TODO_V2_IMPORT: labels seem to be handled by the plugin api, and stored in v3 structures but
// TODO_V2_IMPORT_CARRY_OVER: labels seem to be handled by the plugin api, and stored in v3 structures but
// they don't seem to be imported when opening a v2 document
// There is a V3 interface for this — CollectionLabels
labels?: {
singleCase?: string
pluralCase?: string
Expand All @@ -89,10 +91,10 @@ export interface ICodapV2Collection {
parent?: number
title?: string | null
type?: "DG.Collection"
// TODO_V2_IMPORT areParentChildLinksConfigured is not imported
// TODO_V2_IMPORT_IGNORE areParentChildLinksConfigured is not imported
// it is true 5,000 times in 2,812 files in cfm-shared
// it is false at least 20,000 times
// it might not be optional
// According to V2 documentation it is no longer used
areParentChildLinksConfigured?: boolean
}
// when exporting a v3 collection to v2
Expand All @@ -115,33 +117,37 @@ export interface ICodapV2ExternalContext {
}
export interface ICodapV2DataContextMetadata {
description?: string
// TODO_V2_IMPORT data context metadata source is not imported
// TODO_V2_IMPORT_CARRY_OVER data context metadata source is not imported
// unknown how many instances in this location
// In V3 this is stored as sourceName in dataset
source?: string
// TODO_V2_IMPORT data context metadata importDate is not imported
// TODO_V2_IMPORT_CARRY_OVER G data context metadata importDate is not imported
// at least 20,000 instances in cfm-shared
importDate?: string
// TODO_V2_IMPORT "import date" is not imported
// TODO_V2_IMPORT_EXTRACT "import date" is not imported
// 2,025 instances in cfm-shared
// Assume that it is the same as importDate and convert
"import date"?: string
}
export interface ICodapV2DataContext {
type: "DG.DataContext"
document?: number // id of containing document
guid: number
// TODO_V2_IMPORT do we need to import id so we can export it
// it again?
// TODO_V2_IMPORT_STORE do we need to import id so we can export it again?
// It is not always present in v2 files.
id?: number
// TODO_V2_IMPORT flexibleGroupingChangeFlag is not imported
// TODO_V2_IMPORT_DEFINE_AND_IMPLEMENT flexibleGroupingChangeFlag is not imported
// This is set to true in 11,000 cfm-shared files
// This flag is used in interaction with plugin to indicate that the user
// has moved an attribute in a way that invalidates the plugin's assumptions.
// V3 should define and implement, following the pattern set in V2
flexibleGroupingChangeFlag?: boolean | null
name?: string
title?: string
collections: ICodapV2Collection[]
description?: string
metadata?: ICodapV2DataContextMetadata | null
// TODO_V2_IMPORT preventReorg is not imported
// TODO_V2_IMPORT_DEFINE_AND_IMPLEMENT preventReorg is not imported
// it is false at least 20,000 times
// it is true 969 times in 316 files
preventReorg?: boolean
Expand All @@ -153,7 +159,7 @@ export interface ICodapV2DataContext {
// 259 of these arrays have a first object that doesn't have an "id" field
setAsideItems?: ICodapV2SetAsideItem[] | Record<string, number | string>[]
contextStorage: ICodapV2DataContextStorage
// TODO_V2_IMPORT we are ignoring _permissions. It seems like a CFM
// TODO_V2_IMPORT_IGNORE we are ignoring _permissions. It seems like a CFM
// artifact
_permissions?: any
}
Expand All @@ -165,9 +171,10 @@ export interface ICodapV2DataContextSelectedCase {

export interface ICodapV2DataContextStorage {
_links_?: {
// TODO_V2_IMPORT selectedCases is not imported
// TODO_V2_IMPORT_EXTRACT selectedCases is not imported
// they appear at lest 20,000 times perhaps both in game context and data context
// The value is an empty array 11,300 times
// Note that V3 should be storing selection in V3 documents as well
selectedCases: ICodapV2DataContextSelectedCase[]
}
}
Expand All @@ -190,6 +197,7 @@ export interface ICodapV2GameContextStorage extends ICodapV2DataContextStorage {
// cfm-shared/0b5715a7dab0a92ef332c8407bf51c53cc3ae710/file.json
// It has gameState in the GameContext
// It restores the gameState in v2
// NOTE: The plan is to reimplement Markov and other "legacy" data games plugins to use current API
//
// An example with Next Gen MW games:
// cfm-shared/0b65185859c238170055bde1fef60830e52bd63d49bec96e0b1db84c65ea3356/file.json
Expand Down Expand Up @@ -237,7 +245,7 @@ export interface ICodapV2BaseComponentStorage {
name?: string
userSetTitle?: boolean
// in a document saved by build 0441 this property didn't exist
// TODO_V2_IMPORT: this property seems to be ignored by the import code
// TODO_V2_IMPORT_CARRY_OVER: this property seems to be ignored by the import code
// The v3 models do support it, but from what I can tell each component
// importer needs to read this property from componentStorage and then
// set it on the tile snapshot they pass to insertTile
Expand Down Expand Up @@ -284,7 +292,7 @@ export interface ICodapV2TableStorage extends ICodapV2BaseComponentStorage {
// a context
_links_?: {
context: IGuidLink<"DG.DataContextRecord">
// TODO_V2_IMPORT collapsedNodes is not imported
// TODO_V2_IMPORT_CARRY_OVER collapsedNodes is not imported
// it appears 1,518 times in cfm-shared
// none of those times are empty arrays
collapsedNodes?: IGuidLink<"DG.Case"> | IGuidLink<"DG.Case">[]
Expand All @@ -296,21 +304,21 @@ export interface ICodapV2TableStorage extends ICodapV2BaseComponentStorage {
width?: number
}>
title?: string
// TODO_V2_IMPORT isActive is not imported
// TODO_V2_IMPORT_STORE isActive is not imported
// it occurs in close to 11,0000 files in cfm-shared
// these are both in table and case card
// it might not be optional
isActive?: boolean
// TODO_V2_IMPORT rowHeights is not imported
// TODO_V2_IMPORT_DEFINE_AND_IMPLEMENT rowHeights is not imported
// it occurs more than 20,000 times in cfm-shared
// more than 4,200 of those have non-empty arrays
// it might not be optional
rowHeights?: ICodapV2RowHeight[]
// TODO_V2_IMPORT horizontalScrollOffset is not imported
// TODO_V2_IMPORT_CARRY_OVER horizontalScrollOffset is not imported
// it occurs more than 20,000 times in cfm-shared
// more than 20,000 of those times it has a value other than 0
horizontalScrollOffset?: number
// TODO_V2_IMPORT isIndexHidden is not imported
// TODO_V2_IMPORT_DEFINE_AND_IMPLEMENT isIndexHidden is not imported
// it occurs more than 20,000 times in cfm-shared
// it is true 4,346 times
isIndexHidden?: boolean
Expand Down

0 comments on commit 72b1c32

Please sign in to comment.