Analysis cart creation #1025

squirrelo · 2015-03-31T22:19:41Z

This pull request creates the carts for each user and the cart viewing page. Major pieces are:

Default user analyses added to act as carts
Analysis.get_user_default(User) function added to get the default analysis for a user
processed_data.processing_info added to get processing information needed for cart display.
Cart viewing page added at http://localhost:21174/analysis/selected/ Logging in as test@foo.bar using the test database will prepopulate it with a few samples selected so you can view the page. Currently the page can only be accessed through direct URL.

This is the final piece needed before tying everything together from the last few pull requests into a single unified sample selection system.

…nalysis-changes

coveralls · 2015-04-01T00:19:45Z

Coverage decreased (-0.07%) to 79.02% when pulling 68488c7 on squirrelo:cart-analysis-changes into 54f63e9 on biocore:master.

squirrelo · 2015-04-01T04:24:52Z

Ready for review/merge

antgonza · 2015-04-01T10:06:55Z

Note that #1006 also has a 20.sql. My guess is that this should be 21.sql, right?

squirrelo · 2015-04-01T14:55:02Z

If that pull request is merged first, yes. I'll update once it's merged.

squirrelo · 2015-04-01T15:41:44Z

Neither pull request depends on the other, so feel free to review and merge this one first as well.

coveralls · 2015-04-01T21:53:56Z

Coverage decreased (-0.07%) to 79.06% when pulling 90b3de2 on squirrelo:cart-analysis-changes into 9350269 on biocore:master.

…nalysis-changes

coveralls · 2015-04-02T20:28:42Z

Coverage decreased (-0.07%) to 79.15% when pulling bd3d513 on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

josenavas · 2015-04-02T20:36:11Z

qiita_db/analysis.py

@@ -79,6 +79,26 @@ def _status_setter_checks(self, conn_handler):
            raise QiitaDBStatusError("Can't set status away from public!")

    @classmethod
+    def get_user_default(cls, user):


Uhm... from my point of view, is usually better to ask the user: give me your default cart. It will be a instance method rather than a classmethod, which is already a signal of better design...

This allows you to return the actual object already instantiated, as opposed to just the ID and instantiating, saving a DB call. Do we care about the extra DB call?

You are not saving any DB call...

The check if ID exists on Analysis instantiation isn't needed in this case. That's the DB call I'm referring to.

That call is executed when you are doing cls(aid) (It is calling the init)

Oh, didn't realize that. Sure, can make that change then.

squirrelo · 2015-04-02T20:37:02Z

ready for review/merge

josenavas · 2015-04-02T22:15:49Z

qiita_db/data.py

+        static_info = conn_handler.execute_fetchone(sql, (self.id,))
+
+        # Get the info from the dynamic table, including reference used
+        sql = """SELECT * from qiita.{0}


I'm getting confused in this SQL query, it looks like it is enough to do:

SELECT * FROM qiita.{0} JOIN qiita.reference USING (reference_id) WHERE processed_params_id = {1}

I think this SQL query can return only a single value, since there will be only one row with the given processed_params_id.

Yeah, was being way to protective about the join screwing up. Changed.

josenavas · 2015-04-02T22:34:53Z

Few comments @squirrelo
I will pull it down and test it locally once these issues are solved.

Thanks!

coveralls · 2015-04-02T23:10:22Z

Coverage decreased (-0.07%) to 79.15% when pulling 1dc1440 on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

coveralls · 2015-04-02T23:15:09Z

Coverage decreased (-0.07%) to 79.15% when pulling 1dc1440 on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

squirrelo · 2015-04-02T23:25:44Z

All comments addressed, ready for another round of review.

coveralls · 2015-04-02T23:40:49Z

Coverage decreased (-0.07%) to 79.15% when pulling cf1123f on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

squirrelo · 2015-04-03T16:12:33Z

Can this get a quick final review? It's holding up the remaining work needed to create the cart.

josenavas · 2015-04-03T16:30:51Z

@squirrelo code looks good but the /study/description page is broken. Just log in with the default user and try to see the description of the test study. The page doesn't render. It works on master so it is something on this PR...

josenavas · 2015-04-03T16:42:09Z

@squirrelo I just saw the traceback on the CLI:

ERROR:tornado.application:Future exception was never retrieved: Traceback (most recent call last):
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/gen.py", line 197, in wrapper
    result = func(*args, **kwargs)
  File "/Users/jose/qiime_software/QiiTa/qiita_pet/handlers/study_handlers/description_handlers.py", line 641, in display_template
    prep_tab=prep_tab)
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/web.py", line 664, in render
    html = self.render_string(template_name, **kwargs)
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/web.py", line 771, in render_string
    return t.generate(**namespace)
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/template.py", line 278, in generate
    return execute()
  File "study_description_html.generated.py", line 198, in _tt_execute
    _tt_tmp = _tt_modules.ProcessedDataTab(study, full_access)  # study_description.html:518 (via sitebase.html:192)
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/web.py", line 1429, in render
    rendered = self._active_modules[name].render(*args, **kwargs)
  File "/Users/jose/qiime_software/QiiTa/qiita_pet/uimodules/processed_data_tab.py", line 24, in render
    study_id=study.id)
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/web.py", line 2691, in render_string
    return self.handler.render_string(path, **kwargs)
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/web.py", line 771, in render_string
    return t.generate(**namespace)
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/template.py", line 278, in generate
    return execute()
  File "study_description_templates/processed_data_tab_html.generated.py", line 49, in _tt_execute
    _tt_tmp = _tt_modules.ProcessedDataInfoTab(study_id, pd)  # study_description_templates/processed_data_tab.html:23
  File "/Users/jose/.virtualenvs/qiita/lib/python2.7/site-packages/tornado/web.py", line 1429, in render
    rendered = self._active_modules[name].render(*args, **kwargs)
  File "/Users/jose/qiime_software/QiiTa/qiita_pet/uimodules/processed_data_tab.py", line 56, in render
    process_date = processed_data.processed_date

Probably useful to do a search over processed_date in the entire codebase

squirrelo · 2015-04-03T16:49:03Z

Did, just missed that one. Good catch.

josenavas · 2015-04-03T17:01:50Z

qiita_pet/templates/analysis_selected.html

+	  <h2><a href="/study/description/{{sid}}">{{study.title}}</a></h2>
+	  <table class='table table-striped' id='study{{sid}}-table'>
+	  <tr>
+	  	<th></th><th>id</th><th>Datatype</th><th>Processed Date</th><th>Algorithm</th><th>Reference</th><th></th>


Can we change the id by Processed data id? The issue is that in the actual page we don't know what is that id for, is not completely clear...

Also, is it possible to add a modal that will show the full list of parameters used to process data processed data? I can see this really useful to further samples/processed data filtering

Can we move the show/hide samples at the end of the table, is more natural IMOO

I had it there, but the samples come up on the very left so it's more natural to hit the link and then immediately scroll down.

I can add the rest of the changes though

Sorry to disagree about your previous statement. What about this layout?

coveralls · 2015-04-03T17:08:28Z

Coverage decreased (-0.07%) to 79.15% when pulling 15c71dc on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

squirrelo · 2015-04-03T18:28:06Z

Interface changes made, ready for another review.

josenavas · 2015-04-03T18:31:36Z

I think you did not see my comment. Posting the layout here again:

coveralls · 2015-04-03T18:45:59Z

Coverage decreased (-0.07%) to 79.15% when pulling 8e129e6 on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

coveralls · 2015-04-03T19:20:41Z

Coverage decreased (-0.07%) to 79.15% when pulling d7b4778 on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

coveralls · 2015-04-03T19:28:44Z

Coverage decreased (-0.07%) to 79.15% when pulling d7b4778 on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

squirrelo · 2015-04-03T19:31:47Z

All set for another round.

josenavas · 2015-04-03T20:44:41Z

Thanks @squirrelo one more comment. Instead of adding the info gif that you used, can you use glyphicon glyphicon-info-sign? I think it will be more consistent with other icons on the system

squirrelo · 2015-04-03T20:57:03Z

Oh, yeah that makes sense.

coveralls · 2015-04-03T21:28:31Z

Coverage decreased (-0.13%) to 79.09% when pulling ba42c6a on squirrelo:cart-analysis-changes into 9068d4c on biocore:master.

squirrelo · 2015-04-03T21:33:44Z

Closing to open as a pull request on a separate branch.

squirrelo added 12 commits March 20, 2015 15:20

skeleton for demoing

90339fd

add conformation to removing proc data

72267a0

Merge branch 'master' of https://github.com/biocore/qiita into cart-a…

10f077d

…nalysis-changes

add patch to create default analyses for all existing users

f1ca570

add default analysis on user creation

e362c25

streamline UI for cart

cfe9ba2

further refining IU

28f8480

add default analyses using only SQL

f0c3f80

changes to tests to reflect patch

6d30936

fix more tests, add analysis_workflow steps for carts

4f3756f

update user private_analyses to ignore default cart

b5bf4ed

update test again to reflect change

68488c7

squirrelo added the database changes label Apr 1, 2015

merge upstream/master

90b3de2

Merge branch 'master' of https://github.com/biocore/qiita into cart-a…

bd3d513

…nalysis-changes

josenavas reviewed Apr 2, 2015
View reviewed changes

squirrelo added 2 commits April 2, 2015 13:55

move default analysis pull to user object

f5026f8

implement the default_analysis in qiita_pet

874632b

josenavas reviewed Apr 2, 2015
View reviewed changes

squirrelo added 2 commits April 2, 2015 15:50

more comments addressed

ddf2ff4

pep8

1dc1440

remove magic numbers from tests

cf1123f

replace processed_date retriveal

15c71dc

josenavas reviewed Apr 3, 2015
View reviewed changes

add info modal for proc data

8e129e6

squirrelo added 2 commits April 3, 2015 12:04

more UI changes

e784006

couple small UI changes

d7b4778

use info glyph

ba42c6a

squirrelo closed this Apr 3, 2015

Analysis cart creation #1025

Analysis cart creation #1025

Uh oh!

Conversation

squirrelo commented Mar 31, 2015

Uh oh!

coveralls commented Apr 1, 2015

Uh oh!

squirrelo commented Apr 1, 2015

Uh oh!

antgonza commented Apr 1, 2015

Uh oh!

squirrelo commented Apr 1, 2015

Uh oh!

squirrelo commented Apr 1, 2015

Uh oh!

coveralls commented Apr 1, 2015

Uh oh!

coveralls commented Apr 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

squirrelo commented Apr 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josenavas commented Apr 2, 2015

Uh oh!

coveralls commented Apr 2, 2015

Uh oh!

coveralls commented Apr 2, 2015

Uh oh!

squirrelo commented Apr 2, 2015

Uh oh!

coveralls commented Apr 2, 2015

Uh oh!

squirrelo commented Apr 3, 2015

Uh oh!

josenavas commented Apr 3, 2015

Uh oh!

josenavas commented Apr 3, 2015

Uh oh!

squirrelo commented Apr 3, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Apr 3, 2015

Uh oh!

squirrelo commented Apr 3, 2015

Uh oh!

josenavas commented Apr 3, 2015

Uh oh!

coveralls commented Apr 3, 2015

Uh oh!

coveralls commented Apr 3, 2015

Uh oh!

coveralls commented Apr 3, 2015

Uh oh!