search

biocore / empress

A fast and scalable phylogenetic tree viewer for microbiome data analysis
BSD 3-Clause "New" or "Revised" License
48 stars 31 forks source link

Allow explicitly passing data to make-dev-page.py without including feature metadata #539

Closed fedarko closed 3 years ago

fedarko commented 3 years ago

Noticed while going through #530, although this isn't really in scope of that!

The make-dev-page script uses the condition of "if one of {tree, table, sample metadata} is None" to determine when to load the default moving pictures data. Since feature metadata isn't checked, this means that the bottom line in the code below will fail with AttributeError: 'NoneType' object has no attribute 'to_dataframe' when only passing the tree, table, and sample metadata to the script:

https://github.com/biocore/empress/blob/3d60a2941bea3837dc76750f305b20e212a9c828/tests/python/make-dev-page.py#L46-L71

This PR fixes this, allowing the generation of dev pages without feature metadata.

One thing worth noting is that I don't know if it's possible currently (using make-dev-page.py) to pass an ordination without passing in feature metadata, or to pass in feature metadata without passing in a table / sample metadata -- since these arguments seem to be based on position. (It's totally possible to do this sort of stuff when actually running Empress normally, as shown in e.g. #485, but IDK if replicating this situation is possible in make-dev-page.py. It's not a big deal since this script is only used for testing, though.)

fedarko commented 3 years ago

Seems like uploading to McHelper failed again? Just tried to rerun the job, which should make it a bit clearer if this is an intermittent issue or not.

emperor-helper commented 3 years ago

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

ElDeveloper commented 3 years ago

Looks like it is an intermitent issue. Thanks so much for the PR!

On Jul 29, 2021, at 6:05 PM, Penguin McHelper @.***> wrote:

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

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

ElDeveloper commented 3 years ago

Thanks @fedarko