biocore / redbiom

Sample search by metadata and features
Other
44 stars 20 forks source link

contexts parameter as a list of str #69

Closed antgonza closed 1 year ago

antgonza commented 5 years ago

It would be nice if all methods that require a context could match that requirement and receive a list of str vs just a str.

Parameters

context : str The context to operate in taxon : str The taxon to search for get : function, optional A get method

- List of contexts: Help says that it gets multiple context but when testing passing a list or looping over it and passing individual contexts yield different results

? redbiom.util.ids_from Signature: redbiom.util.ids_from(it, exact, axis, contexts) Docstring: Grab samples from an iterable of IDs

Parameters

it : iteraable of str The IDs to search for exact : boolean If True, compute the intersection of results per context. If False, compute the union of results per context. axis : {'feature', 'sample'} The axis to operate over. contexts : list of str The contexts to search in

wasade commented 5 years ago

I disagree, this would complicate the API as consumers of the methods would then need to disentangle what identifiers are associated with the respective context. I'm not finding any instance of the redbiom codebase that utilizes list of context either...

@antgonza: "List of contexts: Help says that it gets multiple context but when testing passing a list or looping over it and passing individual contexts yield different results" -> that sounds surprising, do you have an example?

antgonza commented 5 years ago
wasade commented 5 years ago

What's being proposed is a change to a very large portion of the API. This will require a substantial and likely complex pull request. The motivation for this request is to remove simple for loops in Qiita. Is that assessment accurate?

I'm unable to reproduce the problem eluded too (see below). Would it be possible to provide an example of the problem encountered?

>>> import redbiom.fetch
>>> import redbiom.util
>>> context_a = 'Deblur-Illumina-16S-V1-2-150nt-780653'
>>> context_b = 'Deblur-Illumina-16S-V45-100nt-fbc5b2'
>>> context_a_sample_ids = redbiom.fetch.samples_in_context(context_a, unambiguous=True)
>>> context_b_sample_ids = redbiom.fetch.samples_in_context(context_b, unambiguous=True)
>>> observed_a = redbiom.util.ids_from(context_a_sample_ids, False, 'sample', [context_a])
>>> observed_b = redbiom.util.ids_from(context_b_sample_ids, False, 'sample', [context_b])
>>> observed_combined = redbiom.util.ids_from(context_a_sample_ids | context_b_sample_ids, False, 'sample', [context_a, context_b])
>>> assert (observed_a | observed_b) == observed_combined
wasade commented 1 year ago

I'm going to close this as I believe the for loop satisfies the need.