refactor: Simplify mapsheet calculation code TDE-1010 (#840)

#### Motivation Simplify the code to the point where TDE-1010 becomes easier to implement. #### Modification Renames a bunch of things for clarity, and removes some dead code. #### Checklist - [x] Tests updated - [ ] Docs updated (N/A) - [x] Issue linked in Title
linz · Jan 31, 2024 · 02a8baf · 02a8baf
1 parent b0a7874
commit 02a8baf
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 109 deletions.
diff --git a/src/commands/tileindex-validate/__test__/tileindex.validate.data.ts b/src/commands/tileindex-validate/__test__/tileindex.validate.data.ts
@@ -33,12 +33,12 @@ export class FakeCogTiff extends CogTiff {
   }
 
   static fromTileName(tileName: string): FakeCogTiff {
-    const extract = MapSheet.extract(tileName);
-    if (extract == null) throw new Error('invalid tile name: ' + tileName);
+    const mapTileIndex = MapSheet.getMapTileIndex(tileName);
+    if (mapTileIndex == null) throw new Error('invalid tile name: ' + tileName);
 
     return new FakeCogTiff(`s3://path/${tileName}.tiff`, {
-      origin: [extract.origin.x, extract.origin.y],
-      size: { width: extract.width, height: extract.height },
+      origin: [mapTileIndex.origin.x, mapTileIndex.origin.y],
+      size: { width: mapTileIndex.width, height: mapTileIndex.height },
       epsg: 2193,
     });
   }

diff --git a/src/commands/tileindex-validate/__test__/tileindex.validate.test.ts b/src/commands/tileindex-validate/__test__/tileindex.validate.test.ts
@@ -17,10 +17,10 @@ import { FakeCogTiff } from './tileindex.validate.data.js';
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
-function convertTileName(x: string, scale: number): string | null {
-  const extract = MapSheet.extract(x);
-  if (extract == null) return null;
-  return getTileName(extract.bbox[0], extract.bbox[3], scale);
+function convertTileName(fileName: string, scale: number): string | null {
+  const mapTileIndex = MapSheet.getMapTileIndex(fileName);
+  if (mapTileIndex == null) return null;
+  return getTileName(mapTileIndex.bbox[0], mapTileIndex.bbox[3], scale);
 }
 
 describe('getTileName', () => {

diff --git a/src/commands/tileindex-validate/tileindex.validate.ts b/src/commands/tileindex-validate/tileindex.validate.ts
@@ -182,9 +182,9 @@ export const commandTileIndexValidate = command({
         features: [...outputs.values()].map((locs) => {
           const firstLoc = locs[0];
           if (firstLoc == null) throw new Error('Unable to extract tiff locations from: ' + args.location);
-          const extract = MapSheet.extract(firstLoc.tileName);
-          if (extract == null) throw new Error('Failed to extract tile information from: ' + firstLoc.tileName);
-          return Projection.get(2193).boundsToGeoJsonFeature(Bounds.fromBbox(extract.bbox), {
+          const mapTileIndex = MapSheet.getMapTileIndex(firstLoc.tileName);
+          if (mapTileIndex == null) throw new Error('Failed to extract tile information from: ' + firstLoc.tileName);
+          return Projection.get(2193).boundsToGeoJsonFeature(Bounds.fromBbox(mapTileIndex.bbox), {
             source: locs.map((l) => l.source),
             tileName: firstLoc.tileName,
           });
@@ -276,39 +276,38 @@ export interface TiffLocation {
  */
 export async function extractTiffLocations(
   tiffs: CogTiff[],
-  scale: number,
+  gridSize: number,
   forceSourceEpsg?: number,
 ): Promise<TiffLocation[]> {
   const result = await Promise.all(
-    tiffs.map(async (f): Promise<TiffLocation | null> => {
+    tiffs.map(async (tiff): Promise<TiffLocation | null> => {
       try {
-        const bbox = await findBoundingBox(f);
-        if (bbox == null) throw new Error(`Failed to find Bounding Box/Origin: ${f.source.url}`);
+        const bbox = await findBoundingBox(tiff);
 
-        const sourceEpsg = forceSourceEpsg ?? f.images[0]?.epsg;
-        if (sourceEpsg == null) throw new Error(`EPSG is missing: ${f.source.url}`);
+        const sourceEpsg = forceSourceEpsg ?? tiff.images[0]?.epsg;
+        if (sourceEpsg == null) throw new Error(`EPSG is missing: ${tiff.source.url}`);
         const centerX = (bbox[0] + bbox[2]) / 2;
         const centerY = (bbox[1] + bbox[3]) / 2;
         // bbox is not epsg:2193
         const targetProjection = Projection.get(2193);
         const sourceProjection = Projection.get(sourceEpsg);
 
         const [x, y] = targetProjection.fromWgs84(sourceProjection.toWgs84([centerX, centerY]));
-        if (x == null || y == null) throw new Error(`Failed to reproject point: ${f.source.url}`);
+        if (x == null || y == null) throw new Error(`Failed to reproject point: ${tiff.source.url}`);
         // Tilename from center
-        const tileName = getTileName(x, y, scale);
+        const tileName = getTileName(x, y, gridSize);
 
         // if (shouldValidate) {
         //   // Is the tiff bounding box the same as the mapsheet bounding box!
         //   // Also need to allow for ~1.5cm of error between bounding boxes.
         //   // assert bbox == MapSheet.extract(tileName).bbox
         // }
-        return { bbox, source: f.source.url.href, tileName, epsg: f.images[0]?.epsg };
+        return { bbox, source: tiff.source.url.href, tileName, epsg: tiff.images[0]?.epsg };
       } catch (e) {
-        console.log(f.source.url, e);
+        console.log(tiff.source.url, e);
         return null;
       } finally {
-        await f.source.close?.();
+        await tiff.source.close?.();
       }
     }),
   );
@@ -322,36 +321,38 @@ export function getSize(extent: [number, number, number, number]): Size {
 }
 
 export function validateTiffAlignment(tiff: TiffLocation, allowedError = 0.015): true | Error {
-  const extract = MapSheet.extract(tiff.tileName);
-  if (extract == null) throw new Error('Failed to extract bounding box from: ' + tiff.tileName);
+  const mapTileIndex = MapSheet.getMapTileIndex(tiff.tileName);
+  if (mapTileIndex == null) throw new Error('Failed to extract bounding box from: ' + tiff.tileName);
   // Top Left
-  const errX = Math.abs(tiff.bbox[0] - extract.bbox[0]);
-  const errY = Math.abs(tiff.bbox[3] - extract.bbox[3]);
+  const errX = Math.abs(tiff.bbox[0] - mapTileIndex.bbox[0]);
+  const errY = Math.abs(tiff.bbox[3] - mapTileIndex.bbox[3]);
   if (errX > allowedError || errY > allowedError)
     return new Error(`The origin is invalid x:${tiff.bbox[0]}, y:${tiff.bbox[3]} source:${tiff.source}`);
 
   // TODO do we validate bottom right
   const tiffSize = getSize(tiff.bbox);
-  if (tiffSize.width !== extract.width)
-    return new Error(`Tiff size is invalid width:${tiffSize.width}, expected:${extract.width} source:${tiff.source}`);
-  if (tiffSize.height !== extract.height)
+  if (tiffSize.width !== mapTileIndex.width)
     return new Error(
-      `Tiff size is invalid height:${tiffSize.height}, expected:${extract.height} source:${tiff.source}`,
+      `Tiff size is invalid width:${tiffSize.width}, expected:${mapTileIndex.width} source:${tiff.source}`,
+    );
+  if (tiffSize.height !== mapTileIndex.height)
+    return new Error(
+      `Tiff size is invalid height:${tiffSize.height}, expected:${mapTileIndex.height} source:${tiff.source}`,
     );
   return true;
 }
 
-export function getTileName(originX: number, originY: number, grid_size: number): string {
-  if (!MapSheet.gridSizes.includes(grid_size)) {
+export function getTileName(originX: number, originY: number, gridSize: number): string {
+  if (!MapSheet.gridSizes.includes(gridSize)) {
     throw new Error(`The scale has to be one of the following values: ${MapSheet.gridSizes}`);
   }
 
-  const scale = Math.floor(MapSheet.gridSizeMax / grid_size);
-  const tile_width = Math.floor(MapSheet.width / scale);
-  const tile_height = Math.floor(MapSheet.height / scale);
-  let nb_digits = 2;
-  if (grid_size === 500) {
-    nb_digits = 3;
+  const tilesPerMapSheet = Math.floor(MapSheet.gridSizeMax / gridSize);
+  const tileWidth = Math.floor(MapSheet.width / tilesPerMapSheet);
+  const tileHeight = Math.floor(MapSheet.height / tilesPerMapSheet);
+  let nbDigits = 2;
+  if (gridSize === 500) {
+    nbDigits = 3;
   }
 
   if (!(SHEET_MIN_X <= originX && originX <= SHEET_MAX_X)) {
@@ -362,16 +363,16 @@ export function getTileName(originX: number, originY: number, grid_size: number)
   }
 
   // Do some maths
-  const offset_x = Math.round(Math.floor((originX - MapSheet.origin.x) / MapSheet.width));
-  const offset_y = Math.round(Math.floor((MapSheet.origin.y - originY) / MapSheet.height));
-  const max_y = MapSheet.origin.y - offset_y * MapSheet.height;
-  const min_x = MapSheet.origin.x + offset_x * MapSheet.width;
-  const tile_x = Math.round(Math.floor((originX - min_x) / tile_width + 1));
-  const tile_y = Math.round(Math.floor((max_y - originY) / tile_height + 1));
+  const offsetX = Math.round(Math.floor((originX - MapSheet.origin.x) / MapSheet.width));
+  const offsetY = Math.round(Math.floor((MapSheet.origin.y - originY) / MapSheet.height));
+  const maxY = MapSheet.origin.y - offsetY * MapSheet.height;
+  const minX = MapSheet.origin.x + offsetX * MapSheet.width;
+  const tileX = Math.round(Math.floor((originX - minX) / tileWidth + 1));
+  const tileY = Math.round(Math.floor((maxY - originY) / tileHeight + 1));
 
   // Build name
-  const letters = Object.keys(SheetRanges)[offset_y];
-  const sheet_code = `${letters}${`${offset_x}`.padStart(2, '0')}`;
-  const tile_id = `${`${tile_y}`.padStart(nb_digits, '0')}${`${tile_x}`.padStart(nb_digits, '0')}`;
-  return `${sheet_code}_${grid_size}_${tile_id}`;
+  const letters = Object.keys(SheetRanges)[offsetY];
+  const sheetCode = `${letters}${`${offsetX}`.padStart(2, '0')}`;
+  const tileId = `${`${tileY}`.padStart(nbDigits, '0')}${`${tileX}`.padStart(nbDigits, '0')}`;
+  return `${sheetCode}_${gridSize}_${tileId}`;
 }
diff --git a/src/utils/__test__/geotiff.test.ts b/src/utils/__test__/geotiff.test.ts
@@ -73,14 +73,11 @@ describe('geotiff', () => {
     assert.deepEqual(bbox, [1460800, 5079120, 1461040, 5079480]);
   });
 
-  const fakeSource: Source = { url: new URL('memory://BX20_500_023098.tif'), fetch: async () => new ArrayBuffer(1) };
-  it('should not parse a tiff with no information ', async () => {
+  const url = new URL('memory://BX20_500_023098.tif');
+  const fakeSource: Source = { url: url, fetch: async () => new ArrayBuffer(1) };
+  it('should not parse a tiff with no information ', () => {
     // tiff with no location information and no TFW
-    const bbox = await findBoundingBox({
-      source: fakeSource,
-      images: [],
-    } as unknown as CogTiff);
-    assert.deepEqual(bbox, null);
+    assert.rejects(() => findBoundingBox({ source: fakeSource, images: [] } as unknown as CogTiff));
   });
 
   it('should parse a tiff with TFW', async () => {

diff --git a/src/utils/__test__/mapsheet.test.ts b/src/utils/__test__/mapsheet.test.ts
@@ -6,7 +6,7 @@ import { MapSheetData } from './mapsheet.data.js';
 
 describe('MapSheets', () => {
   it('should extract mapsheet', () => {
-    assert.deepEqual(MapSheet.extract('2022_CG10_500_080037.tiff'), {
+    assert.deepEqual(MapSheet.getMapTileIndex('2022_CG10_500_080037.tiff'), {
       mapSheet: 'CG10',
       gridSize: 500,
       x: 37,
@@ -19,11 +19,6 @@ describe('MapSheets', () => {
     });
   });
 
-  it('should iterate mapsheets', () => {
-    const sheets = [...MapSheet.iterate()];
-    assert.deepEqual(sheets, ExpectedCodes);
-  });
-
   it('should calculate offsets', () => {
     assert.deepEqual(MapSheet.offset('AS00'), { x: 988000, y: 6234000 });
     assert.deepEqual(MapSheet.offset('AS21'), { x: 1492000, y: 6234000 });
@@ -37,24 +32,23 @@ describe('MapSheets', () => {
     });
   }
 
-  const ExpectedCodes = [...new Set(MapSheetData.map((f) => f.code.slice(0, 2)))];
   const TestBounds = [
     { name: 'CG10_500_079035', bbox: [1236160, 4837560, 1236400, 4837920] },
     { name: 'CG10_500_079036', bbox: [1236400, 4837560, 1236640, 4837920] },
   ] as const;
 
   for (const test of TestBounds) {
     it('should get expected size with file ' + test.name, () => {
-      const extract = MapSheet.extract(test.name) as MapTileIndex;
-      assert.equal(extract.origin.x, test.bbox[0]);
-      assert.equal(extract.origin.y, test.bbox[3]);
-      assert.equal(extract.width, test.bbox[2] - test.bbox[0]);
-      assert.equal(extract.height, test.bbox[3] - test.bbox[1]);
+      const mapTileIndex = MapSheet.getMapTileIndex(test.name) as MapTileIndex;
+      assert.equal(mapTileIndex.origin.x, test.bbox[0]);
+      assert.equal(mapTileIndex.origin.y, test.bbox[3]);
+      assert.equal(mapTileIndex.width, test.bbox[2] - test.bbox[0]);
+      assert.equal(mapTileIndex.height, test.bbox[3] - test.bbox[1]);
     });
 
     it('should get expected bounds with file ' + test.name, () => {
-      const extract = MapSheet.extract(test.name) as MapTileIndex;
-      assert.equal(String(extract.bbox), String(test.bbox));
+      const mapTileIndex = MapSheet.getMapTileIndex(test.name) as MapTileIndex;
+      assert.equal(String(mapTileIndex.bbox), String(test.bbox));
     });
   }
 });
diff --git a/src/utils/geotiff.ts b/src/utils/geotiff.ts
@@ -52,9 +52,9 @@ export const PixelIsPoint = 2;
  *
  * @returns [minX, minY, maxX, maxY] bounding box
  */
-export async function findBoundingBox(tiff: CogTiff): Promise<[number, number, number, number] | null> {
+export async function findBoundingBox(tiff: CogTiff): Promise<[number, number, number, number]> {
   const img = tiff.images[0];
-  if (img == null) return null;
+  if (img == null) throw new Error(`Failed to find bounding box/origin - no images found in file: ${tiff.source.url}`);
   const size = img.size;
 
   // If the tiff has geo location information just read it from the tiff
@@ -78,19 +78,14 @@ export async function findBoundingBox(tiff: CogTiff): Promise<[number, number, n
   // Attempt to read a TFW next to the tiff
   const sourcePath = urlToString(tiff.source.url);
   const tfwPath = sourcePath.slice(0, sourcePath.lastIndexOf('.')) + '.tfw';
-  const tfwData = await fsa.read(tfwPath).catch(() => null);
-  if (tfwData) {
-    const tfw = parseTfw(String(tfwData));
+  const tfwData = await fsa.read(tfwPath);
+  const tfw = parseTfw(String(tfwData));
 
-    const x1 = tfw.origin.x;
-    const y1 = tfw.origin.y;
+  const x1 = tfw.origin.x;
+  const y1 = tfw.origin.y;
 
-    const x2 = x1 + tfw.scale.x * size.width;
-    const y2 = y1 + tfw.scale.y * size.height;
+  const x2 = x1 + tfw.scale.x * size.width;
+  const y2 = y1 + tfw.scale.y * size.height;
 
-    return [Math.min(x1, x2), Math.min(y1, y2), Math.max(x1, x2), Math.max(y1, y2)];
-  }
-
-  // Unable to find any bounding box
-  return null;
+  return [Math.min(x1, x2), Math.min(y1, y2), Math.max(x1, x2), Math.max(y1, y2)];
 }
diff --git a/src/utils/mapsheet.ts b/src/utils/mapsheet.ts
@@ -56,9 +56,6 @@ export type Bounds = Point & Size;
 const charA = 'A'.charCodeAt(0);
 const charS = 'S'.charCodeAt(0);
 
-/** Three sheets codes are not used and should be skipped */
-const Skipped = new Set(['BI', 'BO', 'CI']);
-
 /**
  * Topographic 1:50k map sheet calculator
  *
@@ -99,7 +96,7 @@ export const MapSheet = {
    * MapSheet.extract("BP27_1000_4817.tiff") // { mapSheet: "BP27", gridSize: 1000, x: 17, y:48 }
    * ```
    */
-  extract(fileName: string): MapTileIndex | null {
+  getMapTileIndex(fileName: string): MapTileIndex | null {
     const match = fileName.match(MapSheetRegex);
     if (match == null) return null;
     if (match[1] == null) return null;
@@ -126,7 +123,6 @@ export const MapSheet = {
     if (isNaN(out.gridSize) || isNaN(out.x) || isNaN(out.y)) return null;
 
     const origin = MapSheet.offset(out.mapSheet);
-    if (origin == null) return null;
 
     const tileOffset = MapSheet.tileSize(out.gridSize, out.x, out.y);
     out.origin.x = origin.x + tileOffset.x;
@@ -145,7 +141,7 @@ export const MapSheet = {
    * MapSheet.offset("AZ") // { x: 988000, y: 5982000 }
    * ```
    */
-  offset(sheetCode: string): { x: number; y: number } | null {
+  offset(sheetCode: string): { x: number; y: number } {
     const ms = sheetCode.slice(0, 2);
     const x = Number(sheetCode.slice(2));
 
@@ -170,26 +166,6 @@ export const MapSheet = {
     const offsetY = MapSheet.height * scale;
     return { x: (x - 1) * offsetX, y: (y - 1) * offsetY, width: offsetX, height: offsetY };
   },
-
-  /**
-   * Iterate mapsheet codes
-   * @example
-   * ```typescript
-   * [...MapSheet.iterate()] // [ 'AS', 'AT', 'AU', 'AV', 'AW', 'AX', ... ]
-   * ````
-   */
-  *iterate(): Generator<string> {
-    for (let first = 0; first < 3; first++) {
-      for (let second = 0; second < 26; second++) {
-        if (first === 0 && second < charS - charA) continue;
-        const mapSheet = `${String.fromCharCode(charA + first)}${String.fromCharCode(charA + second)}`;
-        if (Skipped.has(mapSheet)) continue;
-
-        yield mapSheet;
-        if (mapSheet === MapSheet.code.end) return;
-      }
-    }
-  },
 };
 
 // Ranges of valid sheet columns for each sheet row. Keys are the row names, and values are ranges