Creates new GitHub Action for standalone testing #499

gibsramen · 2021-03-18T20:17:27Z

Closes #496

New workflow "Empress Standalone CI" that creates a non-QIIME2 environment and runs tests that do not require and QIIME2 dependencies. test_cli updated to not use the Artifact API.

fedarko

Thanks so much @gibsramen! This is super helpful.

fedarko · 2021-03-19T01:45:55Z

.github/workflows/standalone.yml

+    # we can add more versions of node.js in the future
+    strategy:
+      matrix:
+        node-version: [14.x]


Since we are just running Python tests here I think we can avoid setting up the JS stuff -- this'll save time and energy. However, it might be nice to test multiple python versions with the standalone tests if this is possible. Here's an attempt at that. (Not sure if this will work as intended -- if this change breaks things we can add it in a later PR.)

Suggested change

# we can add more versions of node.js in the future

strategy:

matrix:

node-version: [14.x]

strategy:

matrix:

# based roughly on https://github.com/conda-incubator/setup-miniconda#example-1-basic-usage

python-version: ["3.6", "3.7", "3.8", "3.9"]

fedarko · 2021-03-19T01:46:20Z

.github/workflows/standalone.yml

+
+      - name: Set up Node.js enviroment
+        uses: actions/setup-node@v1
+        with:
+          node-version: 14


Suggested change

- name: Set up Node.js enviroment

uses: actions/setup-node@v1

with:

node-version: 14

fedarko · 2021-03-19T01:55:15Z

.github/workflows/standalone.yml

+      - uses: conda-incubator/setup-miniconda@v2
+        with:
+          activate-environment: empress
+          python-version: 3.6


Based on the suggestion about testing multiple python versions I made above.

Suggested change

python-version: 3.6

# https://github.com/conda-incubator/setup-miniconda#example-1-basic-usage

python-version: ${{ matrix.python-version }}

fedarko · 2021-03-19T01:56:43Z

.github/workflows/standalone.yml

+        run: pip install -e .[all]
+
+      # tests that don't import qiime2 dependencies
+      - name: Run Python tests


Suggested change

- name: Run Python tests

- name: Run (non-QIIME 2) Python tests

Figure we might as well be explicit about it :)

.github/workflows/standalone.yml

fedarko · 2021-03-19T02:15:13Z

tests/python/util.py

+PREFIX_DIR = os.path.join("docs", "moving-pictures")
+
+
+def extracted_artifact_path(dir_name, artifact_loc, file_name):


Two things:

Could you add a docstring for this function? Doesn't have to be anything fancy or long, just at least a brief description of what this does and what the params do.

Unless I'm misinterpreting things, it might make sense to rename this function to extract_q2_artifact_to_path() or something more direct than the current name -- the use of extracted_ makes it seem like this function is just finding a path that already exists, when from what I can tell it's actually creating a path.

fedarko · 2021-03-19T02:18:48Z

tests/python/util.py

+                                              "taxonomy.tsv")
+            fmd = pd.read_csv(tax_loc, sep="\t", index_col=0)
+            md = pd.read_csv(md_loc, sep="\t", index_col=0, skiprows=[1])
+        pass


Suggested change

pass

fedarko · 2021-03-19T02:29:07Z

tests/python/util.py

+
+    Returns
+    -------
+    (tree, table, md, fmd, ordination)
+        tree: Artifact with semantic type Phylogeny[Rooted]
+            Phylogenetic tree.
+        table: Artifact with semantic type FeatureTable[Frequency]
+            Feature table.
+        md: Metadata
+            Sample metadata.
+        fmd: Metadata
+            Feature metadata. (Although this is stored in the repository as a
+            FeatureData[Taxonomy] artifact, we transform it to Metadata.)
+        pcoa: Artifact with semantic type PCoAResults
+            Ordination.


Suggested change

Returns

-------

(tree, table, md, fmd, ordination)

tree: Artifact with semantic type Phylogeny[Rooted]

Phylogenetic tree.

table: Artifact with semantic type FeatureTable[Frequency]

Feature table.

md: Metadata

Sample metadata.

fmd: Metadata

Feature metadata. (Although this is stored in the repository as a

FeatureData[Taxonomy] artifact, we transform it to Metadata.)

pcoa: Artifact with semantic type PCoAResults

Ordination.

Parameters

----------

use_artifact_api: bool, optional (default True)

If True, this will load the artifacts using the QIIME 2 Artifact API,

and the returned objects will have types corresponding to the first

listed types (before the | characters) shown below.

If False, this will instead load the artifacts without using QIIME 2's

APIs; in this case, the returned objects will have types corresponding

to the second listed types (after the | characters) shown below.

Returns

-------

(tree, table, md, fmd, ordination)

tree: qiime2.Artifact | skbio.tree.TreeNode

Phylogenetic tree.

table: qiime2.Artifact | biom.Table

Feature table.

md: qiime2.Metadata | pandas.DataFrame

Sample metadata.

fmd: qiime2.Metadata | pandas.DataFrame

Feature metadata. (Although this is stored in the repository as a

FeatureData[Taxonomy] artifact, we transform it to Metadata if

use_artifact_api is True.)

pcoa: qiime2.Artifact | skbio.OrdinationResults

Ordination.

There may be a more official way to structure this docstring, but I think this makes it pretty clear

tests/python/test_cli.py

fedarko · 2021-03-19T02:48:59Z

tests/python/test_cli.py

-        fm.to_csv(cls.fm_loc, index=True, sep="\t")
-        pcoa.write(cls.pcoa_loc)
+        # extract Artifacts to temporary filesystem
+        # glob used because Artifacts unzip to UUIDs


Suggested change

# glob used because Artifacts unzip to UUIDs

(I think this comment should still be kept around in the codebase, but it should be moved down to the util.extract...() function's docstring)

.github/workflows/standalone.yml

emperor-helper · 2021-03-19T07:01:06Z

The following artifacts were built for this PR: empire-biplot.qzv, empire.qzv, empress-tree.qzv, just-fm.qzv, plain.qzv

fedarko

Everything looks good to me, thanks so much @gibsramen!!!

gibsramen added 7 commits March 18, 2021 12:12

update test_cli to remove Artifact dependency

398a842

add standalone gh actions workflow

e9cb8ee

style

3defb4b

debugging standalone workfow 1/n

24fc7f4

debugging standalone workfow 2/n

8089000

make test_core not reliant on qiime2

0342a54

fix make-dev-page import error

37c05ef

fedarko self-requested a review March 19, 2021 00:32

fedarko mentioned this pull request Mar 19, 2021

Add simple standalone example data #500

Open

fedarko requested changes Mar 19, 2021

View reviewed changes

fedarko reviewed Mar 19, 2021

View reviewed changes

.github/workflows/standalone.yml Show resolved Hide resolved

address comments

5b19570

fedarko approved these changes Mar 19, 2021

View reviewed changes

fedarko merged commit 33ade9c into biocore:master Mar 19, 2021

gibsramen mentioned this pull request Jan 20, 2022

Version 0.0.7 updates biocore/gemelli#53

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creates new GitHub Action for standalone testing #499

Creates new GitHub Action for standalone testing #499

gibsramen commented Mar 18, 2021

fedarko left a comment

fedarko Mar 19, 2021

fedarko Mar 19, 2021

fedarko Mar 19, 2021

fedarko Mar 19, 2021

fedarko Mar 19, 2021

fedarko Mar 19, 2021

fedarko Mar 19, 2021

fedarko Mar 19, 2021

emperor-helper commented Mar 19, 2021

fedarko left a comment

	python-version: 3.6
	# https://github.com/conda-incubator/setup-miniconda#example-1-basic-usage
	python-version: ${{ matrix.python-version }}

	- name: Run Python tests
	- name: Run (non-QIIME 2) Python tests

		PREFIX_DIR = os.path.join("docs", "moving-pictures")


		def extracted_artifact_path(dir_name, artifact_loc, file_name):

Creates new GitHub Action for standalone testing #499

Creates new GitHub Action for standalone testing #499

Conversation

gibsramen commented Mar 18, 2021

fedarko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emperor-helper commented Mar 19, 2021

fedarko left a comment

Choose a reason for hiding this comment