Skip to content
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

Add PClean datasets and schema and run pclean/pclean on them in the integration test #132

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

ThomasColthurst
Copy link
Collaborator

Also, upgrade the integration test to echo the commands being run and (importantly!) fail on any error.

Copy link
Collaborator

@srvasude srvasude left a comment

Choose a reason for hiding this comment

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

Is this schema/observation format backwards incompatible with Model 1-4? I feel pretty out of the loop here with this subdirectory / it's relation to models 1-4.

We have a flights/rents dataset checked in for those models (and in particular have several different schema formats which may make more sense for HIRM). Can we migrate those instead (I think it's confusing now that we have pclean-flights* datasets and flights datasets)?

@ThomasColthurst
Copy link
Collaborator Author

Is this schema/observation format backwards incompatible with Model 1-4? I feel pretty out of the loop here with this subdirectory / it's relation to models 1-4.

Nope, the schema format and observation format are both incompatible with Models 1-4. Models 1-4 are relational, whereas these files are tabular -- the observation format is CSV and the schema file is based on PClean.

We have a flights/rents dataset checked in for those models (and in particular have several different schema formats which may make more sense for HIRM). Can we migrate those instead (I think it's confusing now that we have pclean-flights* datasets and flights datasets)?

We can not. We want to maintain the old relational tests, and add new tests for the pclean/pclean program that wants the new formats.

@ThomasColthurst
Copy link
Collaborator Author

PTAL, as I had to add a bug fix for #139 to this pull request in order to get the integration tests to run successfully.

@ThomasColthurst ThomasColthurst merged commit 64dea8c into master Aug 5, 2024
2 checks passed
@ThomasColthurst ThomasColthurst deleted the 080124-thomaswc-pclean_data branch August 5, 2024 19:41
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