-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Input Schema #207
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
feat: Input Schema #207
Changes from 3 commits
b5c8591
cb89142
4894d0e
2e5f249
f5189e0
4740629
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,73 @@ | ||
| { | ||
| "$schema": "https://specs.frictionlessdata.io/schemas/table-schema.json", | ||
| "fields": [ | ||
| { | ||
| "name": "School Type", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "School Name", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Client Id", | ||
| "type": "string", | ||
| "constraints": { | ||
| "pattern": "^\\d{10}$" | ||
| } | ||
| }, | ||
| { | ||
| "name": "First Name", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Last Name", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Age", | ||
| "type": "integer" | ||
| }, | ||
| { | ||
| "name": "Date of Birth", | ||
| "type": "date" | ||
| }, | ||
| { | ||
| "name": "Street Address Line 1", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Street Address Line 2", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "City", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Province/Territory", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Postal Code", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Overdue Disease", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Overdue Agent", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Imms Given", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "name": "Birth Year", | ||
| "type": "string" | ||
| } | ||
| ], | ||
| "missingValues": [""] | ||
| } |
|
Member
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.
Member
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. Can we snake_case-ify like standards in other pipelines?
Collaborator
Author
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. Went with yaml because the internet suggested it was a little more human readable - so though it might be nicer for other PHUs looking to user our code; but can switch it to json to match our existing pipeline schemas!
Member
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. Ah, I thought frictionless tabular schema was officially only a JSON spec https://specs.frictionlessdata.io/table-schema/#language I agree it's quite readable
Collaborator
Author
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. Yeahh you're right though, I'll change it - readability argument is weak since non-developers probably wouldn't be editing a schema file anyway, and using proper frictionless descriptor makes other tooling we decide to implement straightforward (thanks for the review btw!!) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| fields: | ||
| - name: School Type | ||
| type: string | ||
| - name: School Name | ||
| type: string | ||
| - name: Client Id | ||
| type: string | ||
| constraints: | ||
| pattern: '^\d{10}$' | ||
| - name: First Name | ||
| type: string | ||
| - name: Last Name | ||
| type: string | ||
| - name: Age | ||
| type: integer | ||
| - name: Date of Birth | ||
| type: date | ||
| - name: Street Address Line 1 | ||
| type: string | ||
| - name: Street Address Line 2 | ||
| type: string | ||
| - name: City | ||
| type: string | ||
| - name: Province/Territory | ||
| type: string | ||
| - name: Postal Code | ||
| type: string | ||
| - name: Overdue Disease | ||
| type: string | ||
| - name: Overdue Agent | ||
| type: string | ||
| - name: Imms Given | ||
| type: string | ||
| - name: Birth Year | ||
| type: string | ||
| missingValues: | ||
| - '' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,38 @@ uv run pre-commit install | |
|
|
||
| Input files must be `.xlsx` format with a single worksheet, extracted from [Panorama PEAR](https://accessonehealth.ca/). | ||
|
|
||
| The pipeline enforces a strict column schema — column names must match exactly (no fuzzy matching). The following columns are **required**: | ||
|
|
||
| | Column name | Notes | | ||
| |---|---| | ||
| | `School Type` | | | ||
|
Member
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 see in test fixtures this included "catholic". I would've expected this was secondary/elementary. Is it also possible however, that type is maintained in our look up table from #194 or perhaps isn't needed at all - and simply any dynamics around separate notices for elementary vs secondary is dealt with via the optional, more generic version code approach (see other comment on optional fields)
Collaborator
Author
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. In current code, school type column is never actually used - templates pass hard-coded values to conf.typ for info box; in the interest of min required cols, this could be easily handled as template/version specific |
||
| | `School Name` | | | ||
| | `Client Id` | 10-digit numeric string | | ||
| | `First Name` | | | ||
| | `Last Name` | | | ||
| | `Age` | Integer | | ||
|
Member
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. Should work to also refine and minimize this set... Age and Birth Year are best as derived in pipeline as needed rather than relied upon from input data. |
||
| | `Date of Birth` | ISO 8601 date (`YYYY-MM-DD`) | | ||
| | `Street Address Line 1` | | | ||
| | `Street Address Line 2` | May be blank | | ||
| | `City` | | | ||
| | `Province/Territory` | | | ||
| | `Postal Code` | | | ||
| | `Overdue Disease` | May be blank | | ||
| | `Overdue Agent` | May be blank | | ||
| | `Imms Given` | May be blank | | ||
| | `Birth Year` | | | ||
|
|
||
| The following columns are **optional** and will be used when present: | ||
|
|
||
| | Column name | | ||
|
Member
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. Support optional "version" identifier (e.g. for multi-language support, A/B tests, etc) |
||
| |---| | ||
| | `Board Name` | | ||
| | `Board Id` | | ||
| | `School Id` | | ||
| | `Unique Id` | | ||
|
Member
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. This is different from client ID?
Collaborator
Author
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. In test sets contains 'SCHOOL NAME - PHIX ID' - perhaps a relic of earlier report formats. This is the structure we currently expect from SCHOOL_NAME column in new reports; will remove 'Unique Id' from expected list and test set to reflect new standards |
||
|
|
||
| The full schema is defined in `config/input_schema.yaml`. If the file is missing any required column, the pipeline will stop immediately with a clear error message listing the missing columns. | ||
|
Member
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. Another idea on this... I am imagining that it's going to come up a lot where we want to list a schema in a user friendly way in documentation, that is also coded in the frictionless JSON and that this will create an opportunity for drift between documentation and implementation. Should we consider (perhaps separate of this PR, or even this project), a tool that will enable us to have an approachable display method in mkdocs for our various schemas?
Collaborator
Author
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. Current approach: |
||
|
|
||
| Place input files in the `input/` subdirectory (not tracked by Git): | ||
|
|
||
| ``` | ||
|
|
||
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.
Is there a reason that we want .xlsx over .csv? .xlsx can do some weird things with dates/formatting we may like to avoid dealing with at all, and incurs some xlsx handling dependencies. Perhaps this is a PEAR thing though - if more rows can be exported at a time in xlsx format vs csv?
I believe with PEAR you can export the report template definitions/schema which we could include in this repo once revised (in addition for PHU users to access the template from our collaboration folder in PEAR)