Skip to content

Ebi improvements pet #1490

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Oct 9, 2015
Merged

Ebi improvements pet #1490

merged 9 commits into from
Oct 9, 2015

Conversation

@josenavas
Copy link
Contributor

Fixes the interface to show that EBI data in the corresponding spot.

@josenavas
Copy link
Contributor Author

Depends on #1489 so review/merge that one first

mortonjt
mortonjt reviewed Oct 8, 2015
View reviewed changes
qiita_db/metadata_template/prep_template.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This is minor - could you rephrase this from "Gets" to "Inquires"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@mortonjt
Copy link
Contributor

mortonjt commented Oct 8, 2015

Don't forget the pep8 error

mortonjt
mortonjt reviewed Oct 8, 2015
View reviewed changes
qiita_ware/ebi.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to pass in samples that aren't contained in self._ebi_sample_accessions? If so, would it be worthwhile to add a check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't. samples is a subset of the samples in the sample template, and _ebi_sample_accessions has an entry for each of the samples in the sample template.

@josenavas
Copy link
Contributor Author

Comments addressed.

ElDeveloper
ElDeveloper reviewed Oct 8, 2015
View reviewed changes
qiita_pet/util.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

ebi_linkifier -> EBI_LINKIFIER

@ElDeveloper
Copy link
Contributor

Just one comment, code looks good otherwise. I'll take a pass through the interface though. @mortonjt, did you review the interface?

@mortonjt
Copy link
Contributor

mortonjt commented Oct 8, 2015

Nope. I'll try to take a look at the interface later today.

Jamie

On Thu, Oct 8, 2015 at 9:54 AM, Yoshiki Vázquez Baeza <
notifications@github.com> wrote:

Just one comment, code looks good otherwise. I'll take a pass through the
interface though. @mortonjt https://github.com/mortonjt, did you review
the interface?

—
Reply to this email directly or view it on GitHub
#1490 (comment).

@antgonza antgonza mentioned this pull request Oct 8, 2015
Merged
14 tasks
@josenavas josenavas mentioned this pull request Oct 8, 2015
Merged
@ElDeveloper
Copy link
Contributor

Thanks @josenavas , 👍 @mortonjt you can merge as soon as you are done testing.

@ElDeveloper ElDeveloper mentioned this pull request Oct 9, 2015
Merged
@mortonjt
Copy link
Contributor

mortonjt commented Oct 9, 2015

👍

ElDeveloper added a commit that referenced this pull request Oct 9, 2015
@ElDeveloper
Merge pull request #1490 from josenavas/ebi-improvements-pet
e261564 
Ebi improvements pet
@ElDeveloper ElDeveloper merged commit e261564 into qiita-spots:ebi-improvements Oct 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@josenavas @mortonjt @ElDeveloper