1084 qiita pet #1207

josenavas · 2015-05-24T17:19:35Z

Built on top of #1206

This PR refactors the interface so now the user has to upload first the prep template before he can add the raw data. This is how the database works and this is the normal workflow when using Qiita.

This is still a work in progress PR. I had to completely rewrite some parts of the interface code, and I've still not removed the old code, as it was useful for reference.

It would be great if people can pull down this branch and check which things are missing in the interface, and which things can be improved. I'll be able to work on it tonight and tomorrow morning, so the entire interface is consistent with the new DB layout. After that, I will be able to finally add the QIIME mapping file parsing, which shouldn't impact the interface and it's only a backend change!

…ad to 'raw_data_tab'

josenavas · 2015-05-25T08:07:06Z

Tests should pass now in this PR. Ready for review once all the previous PR have been reviewed and merged.

coveralls · 2015-05-25T22:31:30Z

Coverage decreased (-0.43%) to 78.59% when pulling f65e9da on josenavas:1084-qiita-pet into 3a0976b on biocore:fix-1084.

…-qiita-pet

josenavas · 2015-05-28T17:20:21Z

@biocore/qiita-dev In this PR I've changed the interface a lot. I would suggest first downloading the code and check the interface before actually doing the code review. The idea is that if the interface needs to change, we don't waste time reviewing code that is going to change.

ElDeveloper · 2015-05-28T17:24:14Z

Thanks for the heads up, will do!

On (May-28-15|10:20), josenavas wrote:

@biocore/qiita-dev In this PR I've changed the interface a lot. I would suggest first downloading the code and check the interface before actually doing the code review. The idea is that if the interface needs to change, we don't waste time reviewing code that is going to change.

Reply to this email directly or view it on GitHub:
#1207 (comment)

squirrelo · 2015-05-28T18:15:02Z

qiita_db/test/test_commands.py

'assertItemsEqual' would be better here, since it would eliminate the sorting.

Other checks in this tests depend on these 2 lists being in the same order. Leaving the sort.

squirrelo · 2015-05-28T18:15:11Z

qiita_db/test/test_data.py

Same as above

squirrelo · 2015-05-28T18:34:21Z

Few comments.

ElDeveloper · 2015-05-28T21:46:06Z

The code seems to work just fine i. e. I was able to create a study and process the sequences up to an OTU table.

ElDeveloper · 2015-05-28T22:12:51Z

qiita_db/test/test_data.py

This should be self._clean_up_files.extend([.....

Good catch, done

antgonza · 2015-05-28T22:46:13Z

This looks really nice and like @ElDeveloper said it works.

Sore general comments:

Do we really need an '+Add raw data' button? Why not always show the 2 frames?
If we keep the previous button, add a space between the + and the Add raw data
In "Use existing raw data" add new lines between the dropbox and the "Add" button
Will it be possible to make the space bigger in the grey-add-raw-data section?

Gonna check the code now.

ElDeveloper · 2015-05-28T22:52:29Z

qiita_pet/uimodules/prep_template_tab.py

I would almost prefer that this loop plus the other loop were done in a single SQL query.

See my comment above. If we introduce finer control over the study visibility, this kind of functions will be added as User attributes.

Got it.

On (May-28-15|19:21), josenavas wrote:

+filepath_types = [k.split('', 1)[1].replace('', ' ')

for k in get_filepath_types()

if k.startswith('raw_')]
+fp_type_by_ft = defaultdict(

lambda: filepath_types, SFF=['sff'], FASTA=['fasta', 'qual'],

FASTQ=['barcodes', 'forward seqs', 'reverse seqs'])

+def get_accessible_raw_data(user):

"""Retrieves a tuple of raw_data_id and the last study title for that

raw_data

"""

d = {}

for sid in user.user_studies:

See my comment above. If we introduce finer control over the study visibility, this kind of functions will be added as User attributes.

Reply to this email directly or view it on GitHub:
https://github.com/biocore/qiita/pull/1207/files#r31294962

ElDeveloper · 2015-05-28T23:26:46Z

The code looks good to me, 👍 after the comments are addressed.

ElDeveloper · 2015-05-29T00:15:36Z

When showing the Add new raw data and Use existing raw data accordions, they resize a bit oddly:

Would be nice if the size was fixed.

The Add button should probably be green instead of blue. It is also a bit confusing that the name of the button is Add, but the instructions are Choose the files to link to the raw data. However, I think this is ok for the Use existing raw data section.

This should probably just be a bootsrap table:

For some extra Yoshiki-points, we probably want to convert the dropdown that's shown in Use existing raw data into a chosen dropdown.

Looks good otherwise!

josenavas · 2015-05-29T03:22:18Z

@ElDeveloper thanks for the suggestions!

I want all the extra Yoshiki-points (worth 🍻 ??). Weirdly enough, the Use existing raw data dropdown is already a chosen dropdown:

<select class="chosen-select" id="previous-raw-data-{{pt_id}}" data-placeholder=" ">

And I cannot find why is not working properly...

coveralls · 2015-05-29T03:44:25Z

Changes Unknown when pulling 10a69d8 on josenavas:1084-qiita-pet into * on biocore:fix-1084*.

ElDeveloper · 2015-05-29T04:32:18Z

qiita_pet/uimodules/prep_template_tab.py

Another thing ... there's no tests for these functions, which I think we are ok as this is a transitive step, but I wanted to highlight this in case someone else has stronger feelings about it.

ElDeveloper · 2015-05-29T04:35:43Z

🍻 indeed, thanks for making the changes!

I think you need to activate the dropdown with JavaScript:
https://harvesthq.github.io/chosen/#setup

On (May-28-15|20:22), josenavas wrote:

@ElDeveloper thanks for the suggestions!

I want all the extra Yoshiki-points (worth 🍻 ??). Weirdly enough, the Use existing raw data dropdown is already a chosen dropdown:
<select class="chosen-select" id="previous-raw-data-{{pt_id}}" data-placeholder=" ">
And I cannot find why is not working properly...

Reply to this email directly or view it on GitHub:
#1207 (comment)

ElDeveloper · 2015-05-29T04:46:38Z

BTW, I'm 👍 for the most part now. @squirrelo, do you have any other comments, or others? Would be great to get this merged ASAP.

squirrelo · 2015-05-29T14:16:30Z

I'm still meh on the accordion for Raw Data, but for the sake of having this functionality in 👍 on this code and we can do interface changes later.

antgonza · 2015-05-29T14:35:27Z

RE accordion: I actually like it.

1084 qiita pet

josenavas added 7 commits May 22, 2015 11:51

Change RawDataTab to PrepTemplateTab

36c01b0

Hooking all related with raw data creation

d51b299

Adding all bits for raw data deletion

2720331

Adding bits for adding an existing raw data to a prep template

c85e180

Adding bits for updating investigation type

7394084

Adding preprocessing bits

f174eef

Merge branch '1084-qiita-ware' into 1084-qiita-pet

5035ba0

josenavas added enhancement GUI labels May 24, 2015

josenavas added this to the Alpha 0.2 milestone May 24, 2015

josenavas added 9 commits May 24, 2015 21:02

Merge branch '1084-prep-template' into 1084-qiita-pet

8488071

Removing left out print code

40043e6

Making sure that all return pages go to the 'prep_template_tab' inste…

412046c

…ad to 'raw_data_tab'

Removing leftover code

5feb3d9

Adding ability to (un)link raw data files

07b7e65

Solving merge conflicts

308ce03

Disabling processing if raw data has no files attached

fdaa7e7

Merge branch '1084-data' into 1084-qiita-pet

8378311

Fixing qiita_pet tests

900a391

josenavas changed the title ~~WIP: 1084 qiita pet [DO NOT MERGE] - but review~~ 1084 qiita pet May 25, 2015

josenavas added 4 commits May 25, 2015 03:15

Fixing test_data_studies/commands.sh

ecd494e

Making sure tests clean the environment

66f72c6

Fixing tests in travis

ee9183d

My ~~friend~~ PEP8...

f65e9da

Merge branch 'fix-1084' of https://github.com/biocore/qiita into 1084…

1258155

…-qiita-pet

squirrelo reviewed May 28, 2015
View reviewed changes

ElDeveloper reviewed May 28, 2015
View reviewed changes

josenavas mentioned this pull request May 28, 2015

1084 qiime map #1219

Merged

josenavas added 3 commits May 28, 2015 17:37

Addressing some comments

0826895

Fixing accordions width

94ca93e

Addressing more comments

10a69d8

ElDeveloper reviewed May 29, 2015
View reviewed changes

antgonza added a commit that referenced this pull request May 29, 2015

Merge pull request #1207 from josenavas/1084-qiita-pet

42326b0

1084 qiita pet

antgonza merged commit 42326b0 into qiita-spots:fix-1084 May 29, 2015

1084 qiita pet #1207

1084 qiita pet #1207

Uh oh!

Conversation

josenavas commented May 24, 2015

Uh oh!

josenavas commented May 25, 2015

Uh oh!

coveralls commented May 25, 2015

Uh oh!

josenavas commented May 28, 2015

Uh oh!

ElDeveloper commented May 28, 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!

squirrelo commented May 28, 2015

Uh oh!

ElDeveloper commented May 28, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antgonza commented May 28, 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!

ElDeveloper commented May 28, 2015

Uh oh!

ElDeveloper commented May 29, 2015

Uh oh!

josenavas commented May 29, 2015

Uh oh!

coveralls commented May 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ElDeveloper commented May 29, 2015

Uh oh!

ElDeveloper commented May 29, 2015

Uh oh!

squirrelo commented May 29, 2015

Uh oh!

antgonza commented May 29, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants