Fix extend functionality #1072
Fix extend functionality #1072
Conversation
|
Changes Unknown when pulling 7324daf on josenavas:fix-extend into * on biocore:master*. |
|
yay!!!!!!! |
|
I'm going to start reviewing this. On (Apr-17-15|14:46), josenavas wrote:
|
| """ | ||
| # Check if we are adding new samples | ||
| sample_ids = md_template.index.tolist() | ||
| sql = """SELECT sample_id FROM qiita.{0} |
There was a problem hiding this comment.
I'm kinda surprised we don't have an ids method for the templates.
There was a problem hiding this comment.
that's what an oversight on my end... we have self.keys(), changed.
|
Looks good, just a couple questions. |
|
And comments. |
|
|
||
| # Check if we are adding new columns | ||
| table_name = self._table_name(self._id) | ||
| # Get the required columns from the DB |
There was a problem hiding this comment.
This is an excessive number of comments, to the point it makes the code hard to read.
|
One comment, non-blocking. |
| # modified (see update for that functionality) | ||
| min_md_template = md_template[new_cols].loc[existing_samples] | ||
| values = as_python_types(min_md_template, new_cols) | ||
| values.append(existing_samples) |
There was a problem hiding this comment.
Can you add an explanation for this and the following line? It's a bit confusing.
|
👍 |
Some background:
The extend functionality was first introduced to add new samples to a sample template. However, if there was a new column, that new column was also added to the database. The problem is that at the beginning of the function, the existing samples were filtered from the metadata template so the existing samples didn't get the values for the new column.
This PR includes: