initial push #1

antgonza · 2025-08-29T15:36:06Z

No description provided.

AmandaBirmingham

A couple of questions and a couple of potential typos. Only requesting changes for the typos (if they are) :)

AmandaBirmingham · 2025-09-03T17:03:48Z

.github/workflows/qiita-plugin-ci.yml

+          POSTGRES_DB: postgres
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          COVER_PACKAGE: ${{ matrix.cover_package }}


where is cover_package set?

This is what happens when you copy file and forget to remove some lines/variables.

AmandaBirmingham · 2025-09-03T17:07:10Z

.github/workflows/qiita-plugin-ci.yml

+        shell: bash -l {0}
+        run: |
+          # seqtk is a conda-installed requirement for metapool and isn't
+          # installed automatically by its setup.py.


While this comment is accurate, I am not clear why it is in this particular spot since I don't see anything obviously seqtk-related near it, such as a seqtk conda install. What am I missing?

also left over.

AmandaBirmingham · 2025-09-03T17:08:45Z

.github/workflows/qiita-plugin-ci.yml

+          export QIITA_CONFIG_FP=`pwd`/qiita-dev/qiita_core/support_files/config_test_local.cfg
+          export PYTHONWARNINGS="ignore:Certificate for localhost has no \`subjectAltName\`"
+
+          pytest qp_pacbio --doctest-modules --cov=qp_pacbio --cov-report=lcov


Curious: does all of qiita use pytest for unit testing? For reasons that I don't know (but I have been going along with in order not to rock the boat), kl-metapool uses nosetests ...

Nosetests is no-more: https://nose.readthedocs.io/en/latest/; thus, as we transition to newer pythons (this one is 3.11) we need to migrate to something else; thus, doing the migration already.

ahhhh, very good to know, thank you!

AmandaBirmingham · 2025-09-03T17:10:27Z

README.rst

+   :align: center
+
+
+.. |Build Status| image:: https://github.com/qiita-spots/qp-pacbio/actions/workflows/qiita-plugin-ci.yml/badge.svg


These are cool :)

TBH this is why I prefer rst as I know how to do those things.

AmandaBirmingham · 2025-09-03T20:04:40Z

data/templates/8.lingenome_withmd.py

+            if m:
+                g = 'G%s' % m.group(1)
+
+        # ext > ncbi > metadata


I think the ">" here is confusing me. extension is trumped by ncbi is trumped by metadata, right?

I prefer to leave this files without changes as this is a copy of a file shared by a knight-lab member.

AmandaBirmingham · 2025-09-03T20:10:16Z

qp_pacbio/tests/test_pacbio.py

+        self._clean_up_files.append(out_dir)
+
+        # this should fail cause we don't have valid data
+        success, ainfo, msg = pacbio_processing(


So at the moment do we only have a test of failure (not yet of success)?

Well, we are testing success but nothing is being created, just yet. This is just an skeleton with everything we need but not yet connected.

AmandaBirmingham · 2025-09-03T20:10:49Z

data/templates/8.lingenome_withmd.py

@@ -0,0 +1,188 @@
+#!/usr/bin/env python3


How do you unit test this script?

Haven't gotten here yet. Still need to connect all these steps.

AmandaBirmingham · 2025-09-03T20:12:05Z

images/PacBioProcessing.png

I think there may be typo(s) in the yellow box. It reads "metamap needs to find/have the .bam in has to have the same filename as the fasq". My guess is that either "in" should be "and" or there is something missing before the "has". Also, is the file type here "fasq" or is this a typo for "fastq"?

AmandaBirmingham · 2025-09-03T20:15:06Z

README.md

I'm curious: why does this repo need a .rst readme rather than a .md one?

No reason, just that I'm used to rst

antgonza

Thank you @AmandaBirmingham

antgonza · 2025-09-03T20:18:36Z

.github/workflows/qiita-plugin-ci.yml

+          POSTGRES_DB: postgres
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          COVER_PACKAGE: ${{ matrix.cover_package }}


This is what happens when you copy file and forget to remove some lines/variables.

antgonza · 2025-09-03T20:19:21Z

.github/workflows/qiita-plugin-ci.yml

+        shell: bash -l {0}
+        run: |
+          # seqtk is a conda-installed requirement for metapool and isn't
+          # installed automatically by its setup.py.


also left over.

antgonza · 2025-09-03T20:20:54Z

.github/workflows/qiita-plugin-ci.yml

+          export QIITA_CONFIG_FP=`pwd`/qiita-dev/qiita_core/support_files/config_test_local.cfg
+          export PYTHONWARNINGS="ignore:Certificate for localhost has no \`subjectAltName\`"
+
+          pytest qp_pacbio --doctest-modules --cov=qp_pacbio --cov-report=lcov


Nosetests is no-more: https://nose.readthedocs.io/en/latest/; thus, as we transition to newer pythons (this one is 3.11) we need to migrate to something else; thus, doing the migration already.

antgonza · 2025-09-03T20:21:21Z

README.md

No reason, just that I'm used to rst

antgonza · 2025-09-03T20:21:44Z

README.rst

+   :align: center
+
+
+.. |Build Status| image:: https://github.com/qiita-spots/qp-pacbio/actions/workflows/qiita-plugin-ci.yml/badge.svg


TBH this is why I prefer rst as I know how to do those things.

antgonza · 2025-09-03T20:22:19Z

data/templates/8.lingenome_withmd.py

@@ -0,0 +1,188 @@
+#!/usr/bin/env python3


Haven't gotten here yet. Still need to connect all these steps.

antgonza · 2025-09-03T20:24:02Z

data/templates/8.lingenome_withmd.py

+            if m:
+                g = 'G%s' % m.group(1)
+
+        # ext > ncbi > metadata


I prefer to leave this files without changes as this is a copy of a file shared by a knight-lab member.

antgonza · 2025-09-03T20:27:14Z

qp_pacbio/tests/test_pacbio.py

+        self._clean_up_files.append(out_dir)
+
+        # this should fail cause we don't have valid data
+        success, ainfo, msg = pacbio_processing(


Well, we are testing success but nothing is being created, just yet. This is just an skeleton with everything we need but not yet connected.

AmandaBirmingham

A couple of questions and comments:

Why does the README need to be a .rst file instead of a .md?

In the workflow image, the little yellow note may have a typo or two in it. It says "metawrap needs to find/have the .bam in has to have the same filename as the fasq"

It seems like there may be something missing between "in" and "has". Also, is "fasq" the file type for these files, or should it be the more usual "fastq"?

The box left of box 4 is titled "Folder 3 tools"--could you clarify for me what this means?

AmandaBirmingham · 2025-09-03T20:45:36Z

.github/workflows/qiita-plugin-ci.yml

+          export QIITA_CONFIG_FP=`pwd`/qiita-dev/qiita_core/support_files/config_test_local.cfg
+          export PYTHONWARNINGS="ignore:Certificate for localhost has no \`subjectAltName\`"
+
+          pytest qp_pacbio --doctest-modules --cov=qp_pacbio --cov-report=lcov


ahhhh, very good to know, thank you!

antgonza added 15 commits August 29, 2025 09:35

initial push

5cfb00c

rm 91.sql - not used for this plugin

2a0015f

tool.setuptools_scm

506fcfe

3.11 -> 3.10

a9b884c

"3.10"

ceef72b

simple license

302765c

BSD-3-Clause

228d230

fix install

66d1f13

configure_qp_pacbio

aed4a8d

scripts -> scripts.py

88b1e96

adding a simple test

09a461e

setuptools<81

db851b6

nose -> pytest

7f29d21

pytest-cov

e77f1ca

pytest qp_pacbio

91a4173

AmandaBirmingham requested changes Sep 3, 2025

View reviewed changes

addressing @AmandaBirmingham comments

7d14257

antgonza commented Sep 3, 2025

View reviewed changes

AmandaBirmingham approved these changes Sep 3, 2025

View reviewed changes

antgonza merged commit 2c280b7 into main Sep 3, 2025
2 checks passed

		:align: center


		.. \|Build Status\| image:: https://github.com/qiita-spots/qp-pacbio/actions/workflows/qiita-plugin-ci.yml/badge.svg

initial push #1

initial push #1

Uh oh!

Conversation

antgonza commented Aug 29, 2025

Uh oh!

AmandaBirmingham left a comment

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!

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!

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!

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!

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!

antgonza left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

AmandaBirmingham left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!