Fix 947

[antgonza]

This adds a nicer visualization of dropped samples in an analysis. This is the base to add #1091, which will be in another PR.

[josenavas]

You can use defaultdict(list) and remove the if statement in lines 154-155

[coveralls]

Coverage decreased (-0.06%) to 79.25% when pulling 11e71a5 on antgonza:fix-947 into 01ac199 on biocore:master.

[josenavas]

I think it is better to use tuples rather than dictionaries in here:

{% for title, num_samples, samples in studies %}

this helps to reduce also the amount of python code.

[josenavas]

Thanks @antgonza this looks way better! Few comments though...

[coveralls]

Coverage decreased (-0.03%) to 79.28% when pulling d57bd97 on antgonza:fix-947 into 01ac199 on biocore:master.

[josenavas]

👍

[adamrp]

Why did you change this line? dropped_samples was already set to analysis.dropped_samples on line 145

[adamrp]

In fact, the if statement on line 147 is not needed here, since this for loop will not run if dropped_samples is empty

[antgonza]

1. analysis.dropped_samples -> dropped_samples: will change, left behind by mistake
2. if statement is necessary as dropped_samples is either a dict or None.

Thanks!

[adamrp]

A couple small comments

[coveralls]

Coverage decreased (-0.03%) to 79.28% when pulling 1bea0cd on antgonza:fix-947 into 01ac199 on biocore:master.

[josenavas]

@adamrp I think you suggested that this if statement can be removed. However, analysis.dropped_samples returns None if there are no dropped samples, instead of an empty list. This will make the call to viewitems to fail because of an AttributeError.

I think this should not block this PR and we should open a new issue for modifying analysis.dropped_samples so it returns consistent types (list). Do you agree?

[adamrp]

Agreed -- thanks for pointing that out, I didn't look carefully at analysis.dropped_samples before commenting.

[josenavas]

Cool! New issue created: #1152