Skip to content

multigas calibration details#2747

Merged
rumachan merged 4 commits into
mainfrom
multigas-docs
May 21, 2026
Merged

multigas calibration details#2747
rumachan merged 4 commits into
mainfrom
multigas-docs

Conversation

@rumachan
Copy link
Copy Markdown
Contributor

Initial entry of multigas calibration details, plus README files

@rumachan rumachan requested review from a team and ozym as code owners May 18, 2026 21:31
Copy link
Copy Markdown
Contributor

@ozym ozym left a comment

Choose a reason for hiding this comment

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

There seems to be two different sets of labels, e.g. the file:

Station,Location,Gas,Concentration,Frequency,DayofWeek,CalTime,ZerTime,Start Date,End Date

vs those in the README.

I suggest some gathering together, e.g.

Station
Location
Gas
Concentration
Frequency
Day Of Week
Calibration Time
Zero Time
Start Date
End Date

  • not 100% sure about "Zero Time" as a label but not too worried..

Also, have a think about adjusting the way of writing times, e.g.

02:10:00 could be 2h10m etc.

This may make it easier to read?

@rumachan
Copy link
Copy Markdown
Contributor Author

Thanks @ozym. The different labels was just my slopping work - changed one but didn't check the other, they now match.
I have changed the field names as you suggested. I agree Zero Time sounds a bit funny, but I've added an description.
As the file would normally be read by code and used to find calibrations/zeros I think 02:10:00 is easier to use than 2h10m, although I agree your suggestion is easier to read. My suggestion is to add HH:MM:SS in the units column to help with clarity, and to the same for Start Date etc.

@rumachan rumachan requested a review from ozym May 19, 2026 02:53
Copy link
Copy Markdown
Contributor

@ozym ozym left a comment

Choose a reason for hiding this comment

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

Only comment is where did the number of the week come from, i.e. 0 == monday etc.

Normally in NZ it's 1 == Monday through to 7 == Sunday (as per the first day of the year being 001).

Although, this is encoded in go via:

const (
	Sunday  = iota
	Monday
	Tuesday
	Wednesday
	Thursday
	Friday
	Saturday
)

which is 0 to 6 with 0 being Sunday (which is also different, but Monday is still 1).

Not fussed really but it is something that needs to be carefully noted for users (and when decoding the expected calibration days).

@ozym
Copy link
Copy Markdown
Contributor

ozym commented May 20, 2026

Actually, thinking about this. Would it be possible to simply use real words. e.g.

Day of Week is one of:

Monday
Tuesday
Wednesday
Thursday
Friday
Saturday

And frequency is:

"Daily", "Weekly", etc

We don't need to make it so cryptic it needs decoding.

@rumachan
Copy link
Copy Markdown
Contributor Author

I had actual day names, and then changed it to numbers as it was cleaner to code with 😆 but I agree names is better for a general audience. I will revert to that.

@ozym
Copy link
Copy Markdown
Contributor

ozym commented May 20, 2026

One other thing, as this is meant to be used for actual automatic processing (rather than a reference), I think it would be better to put the csv files in the top of the calibration tree (and update the README there). e.g.

calibration/multigas/calibrations.csv goes to calibration/multigas.csv

I can then do automatic checks/tests etc.

@rumachan
Copy link
Copy Markdown
Contributor Author

I have made those changes @ozym. I have left a reference to the multigas folder in the README as we'll use that for detection limit information when that is fully sorted, though no folder or file there yet.

@rumachan rumachan requested a review from ozym May 21, 2026 02:06
Copy link
Copy Markdown
Contributor

@ozym ozym left a comment

Choose a reason for hiding this comment

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

I'll do a follow up PR with wrap around code.

@rumachan rumachan merged commit 5175c4b into main May 21, 2026
18 checks passed
@rumachan rumachan deleted the multigas-docs branch May 21, 2026 03:08
@rumachan
Copy link
Copy Markdown
Contributor Author

Thanks Mark. Ping me to review that when you are ready.

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