Skip to content

Update redbiom.html #2377

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

Closed
wants to merge 3 commits into from
Closed

Conversation

adswafford
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Oct 27, 2017

Codecov Report

Merging #2377 into release-candidate will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                 @@
##           release-candidate    #2377   +/-   ##
==================================================
  Coverage              93.77%   93.77%           
==================================================
  Files                    163      163           
  Lines                  18668    18668           
==================================================
  Hits                   17505    17505           
  Misses                  1163     1163

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 0107f97...d1e198e. Read the comment docs.

@@ -208,18 +208,31 @@
<ul>
<li>
Find all samples in which the word infant exists, as well as antibiotics,
where the infants are under a certain number of days old:
"infant &amp; antibiotics where age_days < 30"
where the infants are under a year old:
Copy link
Member

Choose a reason for hiding this comment

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

Remember that this is HTML so it will not render as it's written. If you want a new line you will need to use <br/>, if you want to create a list you could do:

<ul>
   <li>element 1</li>
   <li>element 2</li>
</ul>

and please use &amp; for all the &

@antgonza
Copy link
Member

ping @adswafford

Copy link
Contributor Author

@adswafford adswafford left a comment

Choose a reason for hiding this comment

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

Fixed html based on feedback from @antgonza

Left out where "ph< 7 & emp_release1 == True" because the search didn't produce any results; does the metadata need to be updated or is there a different metadata category used to denote the EMP first release?

@wasade
Copy link
Contributor

wasade commented Oct 30, 2017

"ph< 7 & emp_release1 == True" needs to be "where ph < 7 and emp_release1 == 'True'".

If you're testing against redbiom-rc, then this should work. If you're not testing against redbiom-rc, then it won't work as the state of the database at the old load did not include "emp_release1"

@adswafford
Copy link
Contributor Author

adswafford commented Oct 30, 2017 via email

@wasade
Copy link
Contributor

wasade commented Oct 30, 2017

Not sure, works for me against redbiom-rc:

$ redbiom search metadata "where ph < 7 and emp_release1 == 'True'" | wc -l
1176

@adswafford
Copy link
Contributor Author

adswafford commented Oct 30, 2017 via email

@josenavas
Copy link
Contributor

@adswafford we are currently updating the qiita-rc to do a dry upgrade, to ensure the success of tomorrow's main system update.

@antgonza
Copy link
Member

Figured out the issue: in booleans 'True' != 'true', which shouldn't be an issue but fixing in redbiom will take longer than fixing in Qiita. The problem in Qiita is that we were lower everything which was solved and this is not a problem with other queries, for example:

In [4]: len(redbiom.search.metadata_full("where emp_release1 == 'True'", False))
Out[4]: 22306

In [5]: len(redbiom.search.metadata_full("where emp_release1 == 'True'".lower(), False))
Out[5]: 0

# counter example, where it doesn't matter

In [11]: len(redbiom.search.metadata_full("antibiotics & INfant".lower(), False))
Out[11]: 1566

In [12]: len(redbiom.search.metadata_full("antibiotics & INfant", False))
Out[12]: 1566

Just tested in qiita-rc (deployed the fix) and now where emp_release1 == "True" and where ph < 7 and emp_release1 == 'True' now work.

To fix this PR, I'm gonna close it and issue a new one.

@antgonza
Copy link
Member

closing in favor of: #2382

@antgonza antgonza closed this Oct 31, 2017
@josenavas josenavas deleted the redbiom-help-update branch November 1, 2017 14:25
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.

5 participants