Skip to content
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.

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

IMP: merge fastq_stats_* visualizers into a single action #71

Merged
merged 4 commits into from
Jun 9, 2020

Conversation

angrybee
Copy link
Member

@angrybee angrybee commented Jun 2, 2020

Closes #70

@angrybee angrybee self-assigned this Jun 2, 2020
@thermokarst thermokarst added this to In Review in 2020.6 via automation Jun 2, 2020
@angrybee
Copy link
Member Author

angrybee commented Jun 3, 2020

Hi there,
I need help with the format to import in test_stats.py.

The plugin itself transforms SingleLanePerSamplePairedEndFastqDirFmt and SingleLanePerSampleSingleEndFastqDirFmt to CasavaOneEightSingleLanePerSampleDirFmt. In the unittests it doesn't work like this so I get a TypeError: 'BoundFile' object is not subscriptable because of the different format of the manifest in Type CasavaOneEightSingleLanePerSampleDirFmt.

I would like to import these formats to use the given MANIFEST for both tests. I also tried to transform the input manual to CasavaOneEightSingleLanePerSampleDirFmt with no success.

Any suggestions?

@thermokarst thermokarst moved this from In Review to In Development in 2020.6 Jun 3, 2020
@thermokarst
Copy link
Contributor

Hey @angrybee! The easiest solution is to just invoke the plugin action via the framework (rather than importing the function manually). You can see an example of that here:

class ClusterFeaturesOpenReference(TestPluginBase):
package = 'q2_vsearch.tests'
def setUp(self):
super().setUp()
self.open_reference = \
self.plugin.pipelines['cluster_features_open_reference']
input_sequences_fp = self.get_data_path('dna-sequences-2.fasta')
self.input_sequences = Artifact.import_data('FeatureData[Sequence]',
input_sequences_fp)
ref_sequences_fp = self.get_data_path('ref-sequences-1.fasta')
self.ref_sequences = Artifact.import_data('FeatureData[Sequence]',
ref_sequences_fp)
input_table = biom.Table(np.array([[100, 101, 103],
[1, 1, 2],
[4, 5, 6],
[7, 8, 9],
[99, 98, 97]]),
['feature1', 'feature2', 'feature3',
'feature4', 'feature5'],
['sample1', 'sample2', 'sample3'])
self.input_table = Artifact.import_data('FeatureTable[Frequency]',
input_table)
self.input_sequences_list = _read_seqs(self.input_sequences)
def test_100_percent_clustering(self):
# feature1 clusters into r1 and feature4 clusters into r4 during
# closed-ref clustering; feature2, feature3, and feature5 cluster into
# their own clusters during de-novo clustering.
exp_table = biom.Table(np.array([[100, 101, 103],
[1, 1, 2],
[4, 5, 6],
[7, 8, 9],
[99, 98, 97]]),
['r1', 'feature2', 'feature3', 'r2',
'feature5'],
['sample1', 'sample2', 'sample3'])
with redirected_stdio(stderr=os.devnull):
obs_table, rep_seqs, new_ref_seqs = self.open_reference(
sequences=self.input_sequences, table=self.input_table,
reference_sequences=self.ref_sequences, perc_identity=1.0)

@angrybee
Copy link
Member Author

angrybee commented Jun 5, 2020

Thx @thermokarst for the hint which opened a lot of more questions and now I know way to much about qiime2-code-details :)

For me it was not possible to reuse demux-1 because qiime complained that the data I wanted to import as se-Reads are having forward and reverse reads. So you also did a good job there. Because of this I copied the forward reads to a new folder and changed the MANIFEST there. If anybody has a better solution feel free to tell me.

Otherwise I would say, job done, happy to be reviewed.

@angrybee angrybee marked this pull request as ready for review June 5, 2020 11:22
@angrybee angrybee moved this from In Development to In Review in 2020.6 Jun 5, 2020
@thermokarst thermokarst assigned thermokarst and unassigned angrybee Jun 5, 2020
Copy link
Contributor

@thermokarst thermokarst left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @angrybee!

@thermokarst thermokarst merged commit 9b7c340 into qiime2:master Jun 9, 2020
2020.6 automation moved this from In Review to Changelog Needed Jun 9, 2020
@ChrisKeefe ChrisKeefe moved this from Changelog Needed to Completed in 2020.6 Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
2020.6
  
Completed
Development

Successfully merging this pull request may close these issues.

merge fastq_stats_* visualizers into a single action
2 participants