Skip to content

Fixes 1880 #2247

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

Merged
merged 2 commits into from
Aug 21, 2017
Merged

Fixes 1880 #2247

merged 2 commits into from
Aug 21, 2017

Conversation

josenavas
Copy link
Contributor

Fixes #1880
Basically the problem was in this function:

https://github.com/biocore/qiita/blob/a369cad63827b92e89032c8e12d5865c61f73a5b/qiita_ware/dispatchable.py#L198-L204

The serial calls to extend and update was forcing a validation on the "extend" call first (which raised the warning message) and another on the update, which it corrected the values in the DB (but didn't remove the previous warning).

By creating a function that performs those 2 operations at the same time and issuing a single validation at the end of that function we only raise a warning at the end if there is a problem with the data in the DB at the end of the operation.

@codecov-io
Copy link

codecov-io commented Aug 21, 2017

Codecov Report

Merging #2247 into dev will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #2247      +/-   ##
==========================================
+ Coverage   92.81%   92.82%   +0.01%     
==========================================
  Files         171      171              
  Lines       18906    18914       +8     
==========================================
+ Hits        17547    17557      +10     
+ Misses       1359     1357       -2
Impacted Files Coverage Δ
...ta_db/metadata_template/test/test_prep_template.py 99.81% <ø> (-0.01%) ⬇️
..._db/metadata_template/test/test_sample_template.py 99.84% <100%> (-0.01%) ⬇️
...ita_db/metadata_template/base_metadata_template.py 95.17% <100%> (+0.11%) ⬆️
...t/handlers/api_proxy/tests/test_sample_template.py 98.66% <0%> (+0.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a369cad...d1bbe46. Read the comment docs.

# current_map.index are supersets of md_template.columns and
# md_template.index, respectivelly, so this will not fail
current_map = current_map[
md_template.columns].loc[md_template.index]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to do this way instead of current_map = current_map.loc[md_template.index, md_template.columns]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. I know that sometimes one option is creating a view while the other one returning an actual pandas DataFrame object, and that can raise Warning in some operations. I don't think it actually matters here, but I wanted to minimize code modifications (I'm actually just moving code around in this PR to be able to reuse code easily).

@wasade
Copy link
Contributor

wasade commented Aug 21, 2017

👍

@josenavas
Copy link
Contributor Author

Thanks @wasade / @tanaes - able to merge? (I don't want to do a self high-five 😄 )

@tanaes
Copy link
Contributor

tanaes commented Aug 21, 2017 via email

@josenavas
Copy link
Contributor Author

No problem @tanaes - Yes, @wasade gave the 👍 so you can merge. Thanks!

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.

4 participants