Data issue #125
Data issue #125
Conversation
|
This PR is blocking development in 2 other PR, it would be great if it can be reviewed/merged |
| @@ -269,7 +269,7 @@ def test_create(self): | |||
| "SELECT * FROM qiita.processed_data WHERE processed_data_id=2") | |||
| # processed_data_id, preprocessed_data_id, processed_params_table, | |||
| # processed_params_id, processed_date | |||
| exp = [[2, 1, "processed_params_uclust", 1, self.date]] | |||
| exp = [[2, "processed_params_uclust", 1, self.date]] | |||
There was a problem hiding this comment.
This makes me think: should we add "version" (as in, program version) to the processed_params_* tables? E.g., to make sure we know what version of the program the params were applied to.
There was a problem hiding this comment.
I think that makes sense. I've created an issue to keep track of it: #128
Right now this table only holds the minimal data, but this is something that we will need to visit...
|
Minor comments on the code, but looks good after those small corrections are addressed! Should we open an issue about the version thing, or am I overlooking some other place the version information could be stored? |
| self.params_id, self.filepaths) | ||
| ProcessedData.create("processed_params_", self.params_id, | ||
| self.filepaths, | ||
| preprocessed_data=self.preprocessed_data) |
There was a problem hiding this comment.
I might be misunderstanding something here, but isn't this explicitly testing that those tables don't exist?
There was a problem hiding this comment.
Yes, it is, that is why they're under the context manager. They don't actually need the preprocessed_data params, but this was one of the errors that I was getting and I just changed it fast.
There was a problem hiding this comment.
Ah! I was missing something: I neglected to notice that the name of the test ends in _error! Okay, this looks good then.
|
Thanks @adamrp changes done! |
|
Awesome! thanks @josenavas. 👍 |
|
Can someone else review? @ElDeveloper @squirrelo @teravest @antgonza? This is blocking, and would be good to get finished off; I'd like to work on some of the load_processed_data stuff tonight -- I can get a start without this, but can't get too far |
|
Thanks @teravest! |
This changes the SQL layout to support (pre)processed data without raw/preprocessed data.
It fixes any relevant code involved on thath