Skip to content

Conversation

@bkeepers
Copy link
Member

@bkeepers bkeepers commented Dec 17, 2025

This pull request adds a script to import stations from TICON-4, TIdal CONstants based on GESLA-4 sea-level records.

It includes 4,838 global stations.

$ tools/import-ticon
Done. Created 4838 stations.

TODO:

  • Resolve duplicates between NOAA/TICON. It might make sense to just skip any that are from NOAA in TICON, but I want to compare the list and see if there are any differences.
  • Implement missing constituents in tide-predictor
    • MSQM
    • EP2
    • MTM
    • LAMBDA2
    • MKS2
    • N4
    • S3
    • MA2
    • MB2
    • T3
    • R3
    • RHO1
    • SGM
    • 3L2
    • 3N2
    • 2MS6
    • 2MK5
    • 2MO5

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds functionality to import 4,838 global tide stations from the TICON-4 (TIdal CONstants based on GESLA-4) dataset. The implementation includes a bash script to download the data, a TypeScript module to parse and convert the data to the repository's station JSON schema, and a new utility module for computing tidal datums.

Key changes:

  • New import tooling for TICON-4 dataset with automatic data download and processing
  • New datum computation utilities using synthetic tidal predictions
  • Updated @neaps/tide-predictor dependency to version 0.3.0 to support additional harmonic constituents

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tools/import-ticon Bash script that downloads TICON-4 data and invokes the TypeScript import script
tools/import-ticon.ts TypeScript module that parses TICON-4 CSV files and converts stations to JSON schema
tools/datum.ts New utility module for computing tidal datums from harmonic constituents using synthetic predictions
package.json Updates @neaps/tide-predictor dependency from 0.2.1 to 0.3.0
.gitignore Adds tmp directory to exclude downloaded data from version control

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


function dayMonthYearToDate(date: string) {
const [ day, month, year ] = date.split('/').map((v) => parseInt(v, 10))
if(!day || !month || !year) {
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after 'if' keyword. The code style should be consistent with if statements having a space between 'if' and the opening parenthesis.

Copilot uses AI. Check for mistakes.
Comment on lines 92 to 96
const constituents: HarmonicConstituent[] = rows.map((row) => ({
name: row.con,
amplitude: parseFloat(row.amp) / 100, // convert cm to m
phase: ((parseFloat(row.pha) % 360) + 360) % 360, // lag in degrees; normalize to [0, 360)
speed: constituents_by_name[row.con]?.speed,
}))

Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The speed property could be undefined if the constituent name in row.con is not found in constituents_by_name. This would result in harmonic constituents with undefined speed values. Consider adding validation to skip constituents that aren't recognized or provide a clear error message when a constituent is not found in the lookup table.

Suggested change
const constituents: HarmonicConstituent[] = rows.map((row) => ({
name: row.con,
amplitude: parseFloat(row.amp) / 100, // convert cm to m
phase: ((parseFloat(row.pha) % 360) + 360) % 360, // lag in degrees; normalize to [0, 360)
speed: constituents_by_name[row.con]?.speed,
}))
const constituents: HarmonicConstituent[] = rows.map((row) => {
const constituentMeta = constituents_by_name[row.con]
if (!constituentMeta || typeof constituentMeta.speed !== 'number') {
throw new Error(`Unknown or invalid constituent '${row.con}' in TICON data for station '${rows[0].tide_gauge_name}'`)
}
return {
name: row.con,
amplitude: parseFloat(row.amp) / 100, // convert cm to m
phase: ((parseFloat(row.pha) % 360) + 360) % 360, // lag in degrees; normalize to [0, 360)
speed: constituentMeta.speed,
}
})

Copilot uses AI. Check for mistakes.
throw new Error('No rows to convert')
}

const gesla = metadata[rows[0].tide_gauge_name]
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The metadata lookup at line 90 could be undefined if the tide_gauge_name is not found in the metadata. This would cause a runtime error when trying to access gesla['SITE NAME'] at line 106. Consider adding a check to handle the case where metadata is missing for a station.

Suggested change
const gesla = metadata[rows[0].tide_gauge_name]
const gesla = metadata[rows[0].tide_gauge_name]
if (!gesla) {
throw new Error(`Missing metadata for tide gauge: ${rows[0].tide_gauge_name}`)
}

Copilot uses AI. Check for mistakes.

function dayMonthYearToDate(date: string) {
const [ day, month, year ] = date.split('/').map((v) => parseInt(v, 10))
if(!day || !month || !year) {
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation check at line 76 will fail for day, month, or year values of 0, but these can never occur in valid dates. However, the check doesn't handle NaN values that could result from parsing non-numeric strings. Consider using isNaN() or checking if the parsed values are valid numbers to properly catch parsing errors.

Suggested change
if(!day || !month || !year) {
if (![day, month, year].every((v) => Number.isInteger(v) && v > 0)) {

Copilot uses AI. Check for mistakes.
tools/datum.ts Outdated
Comment on lines 83 to 89
if (times.length === 0) {
throw new Error('times array is empty')
}
const firstTime = times[0]
const lastTime = times[times.length - 1]
if (!firstTime || !lastTime) {
throw new Error('times array is empty')
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check at line 72 already validates that times.length is non-empty, making the subsequent check at line 83 redundant. Additionally, the error messages at lines 84 and 89 both say "times array is empty" when line 89 should indicate a different issue (array contains undefined values). Consider removing the redundant check or providing distinct error messages.

Suggested change
if (times.length === 0) {
throw new Error('times array is empty')
}
const firstTime = times[0]
const lastTime = times[times.length - 1]
if (!firstTime || !lastTime) {
throw new Error('times array is empty')
const firstTime = times[0]
const lastTime = times[times.length - 1]
if (!firstTime || !lastTime) {
throw new Error('times array contains invalid or undefined Date values')

Copilot uses AI. Check for mistakes.
process.stdout.write('.')
}

console.log(`\nDone. Created ${created} files`)
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message says "Created X files" but the counter represents the number of stations processed, which corresponds to the number of files created only if all operations succeeded. Since the script could potentially skip some stations or fail silently in edge cases, consider using a more precise message like "Created X station files" or tracking successful saves explicitly.

Suggested change
console.log(`\nDone. Created ${created} files`)
console.log(`\nDone. Created ${created} station files`)

Copilot uses AI. Check for mistakes.
@bkeepers bkeepers mentioned this pull request Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants