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

cleanup taxonomy code after refactor #2446

Merged
merged 5 commits into from
Feb 7, 2023
Merged

Conversation

bluegenes
Copy link
Contributor

@bluegenes bluegenes commented Jan 13, 2023

Taxonomy Refactor Overview

In an attempt to allow usage of NCBI taxid (motivation: CAMI benchmarking) and alternate hierarchical taxonomic ranks (motivation: LINS), I ended up refactoring the taxonomy code in a four-PR series. Taxonomic summarization results should not change. Minor caveat: I was previously obtaining query_bp in a hacky manner to allow gather <4.4 results. The class methods are more robust, and I'd like to stop supporting gather <4.4 results. To allow this, I had to add the query_bp, ksize, and scaled columns into some testing results to keep tests functioning.

  1. Add new classes for lineage manipulation #2437 modifies LineagePair from a two-item collections.namedtuple to a three-item typing.NamedTuple containing an additional field, taxid, for storing NCBI taxid information. It also introduces classes (BaseLineageInfo, RankLineageInfo), which move lineage manipulation (from lca_utils.py) to class methods in order to support robust summarization across compatible lineages (lineages of same hierarchical ranks). To ensure these can be used as dictionary keys, these classes are frozen.

  2. add tax summarization dataclasses for safety and flexibility #2439 introduces classes that facilitate reading, summarization, and writing of gather results. First, it updates three prior collections.namedtuples to dataclasses used for storing information about the gather query (QueryInfo), summarized gather information for metagenome queries (SummarizedGatherResult) and classification information for genome queries (ClassificationResult). It introduces three new classes for reading and manipulating gather results. GatherRow, is used for reading a each row from a gather file and automatically checking for required columns. TaxResult is used for storing a single row from gather file, optionally (and ideally) with taxonomic information, stored as LineageInfo class from PR 1. QueryTaxResult is used for storing all TaxResults associated with a single query. QueryTaxResult add methods to replicate the summarization previously done within summarize_gather_at in tax_utils.py and the classification thresholding in genome within __main__.py.

  3. Use new tax classes for taxonomic summarization #2443 replaces the actual taxonomic summarization code in tax/__main__.py with code that uses the new classes. Modifies gather loading code to read using GatherRow, TaxResult, and QueryTaxResult.

  4. cleanup taxonomy code after refactor #2446 removes old, unused functions that are rendered redundant by the new classes. Also removes associated tests.

Additional details for this PR (#2446)

  • Delete old functions that aren't used outside of taxonomic summarization + associated tests
    • Including old namedtuples: QueryInf, SumGathInf, ClassInf
  • Make sure any old comments/documentation make it into new code
  • Don't use unnecessary empty () for dataclasses

@codecov
Copy link

codecov bot commented Jan 13, 2023

Codecov Report

Merging #2446 (49aebdd) into use-new-tax-classes (60ded63) will increase coverage by 8.39%.
The diff coverage is 100.00%.

@@                   Coverage Diff                   @@
##           use-new-tax-classes    #2446      +/-   ##
=======================================================
+ Coverage                84.15%   92.54%   +8.39%     
=======================================================
  Files                      132      103      -29     
  Lines                    15755    11935    -3820     
  Branches                  2580     2507      -73     
=======================================================
- Hits                     13258    11045    -2213     
+ Misses                    2180      586    -1594     
+ Partials                   317      304      -13     
Flag Coverage Δ
python 92.54% <100.00%> (+0.71%) ⬆️
rust ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/sourmash/tax/__main__.py 93.77% <100.00%> (ø)
src/sourmash/tax/tax_utils.py 98.07% <100.00%> (+7.45%) ⬆️
src/core/src/lib.rs
src/core/src/sketch/hyperloglog/mod.rs
src/core/src/ffi/storage.rs
src/core/src/sketch/nodegraph.rs
src/core/src/encodings.rs
src/core/src/ffi/minhash.rs
src/core/src/index/mod.rs
src/core/src/errors.rs
... and 21 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bluegenes bluegenes marked this pull request as ready for review January 13, 2023 20:57
@bluegenes
Copy link
Contributor Author

@ctb ready for review as well. Just a cleanup companion to #2443.

src/sourmash/tax/tax_utils.py Outdated Show resolved Hide resolved
@bluegenes
Copy link
Contributor Author

@ctb I'll let you decide if you want this merged or separate for easier #2443 reviewing

@ctb
Copy link
Contributor

ctb commented Jan 14, 2023

@ctb I'll let you decide if you want this merged or separate for easier #2443 reviewing

ok thanks! just leave it as is for the moment, then. appreciated :)

bluegenes added a commit that referenced this pull request Jan 16, 2023
## Taxonomy Refactor Overview

In an attempt to allow usage of NCBI taxid (motivation: CAMI
benchmarking) and alternate hierarchical taxonomic ranks (motivation:
LINS), I ended up refactoring the taxonomy code in a four-PR series.
Taxonomic summarization results should not change. Minor caveat: I was
previously obtaining `query_bp` in a hacky manner to allow gather <4.4
results. The class methods are more robust, and I'd like to stop
supporting gather <4.4 results. To allow this, I had to add the
`query_bp`, `ksize`, and `scaled` columns into some testing results to
keep tests functioning.

1. #2437 modifies
`LineagePair` from a two-item `collections.namedtuple` to a three-item
`typing.NamedTuple` containing an additional field, `taxid`, for storing
NCBI taxid information. It also introduces classes (`BaseLineageInfo`,
`RankLineageInfo`), which move lineage manipulation (from
`lca_utils.py`) to class methods in order to support robust
summarization across compatible lineages (lineages of same hierarchical
ranks). To ensure these can be used as dictionary keys, these classes
are frozen.

2. #2439 introduces classes
that facilitate reading, summarization, and writing of gather results.
First, it updates three prior `collections.namedtuple`s to `dataclasses`
used for storing information about the gather query (`QueryInfo`),
summarized gather information for metagenome queries
(`SummarizedGatherResult`) and classification information for genome
queries (`ClassificationResult`). It introduces three new classes for
reading and manipulating gather results. `GatherRow`, is used for
reading a each row from a gather file and automatically checking for
required columns. `TaxResult` is used for storing a single row from
gather file, optionally (and ideally) with taxonomic information, stored
as `LineageInfo` class from PR 1. `QueryTaxResult` is used for storing
all `TaxResult`s associated with a single query. `QueryTaxResult` add
methods to replicate the summarization previously done within
`summarize_gather_at` in `tax_utils.py` and the classification
thresholding in `genome` within `__main__.py`.

3. #2443 replaces the
actual taxonomic summarization code in `tax/__main__.py` with code that
uses the new classes. Modifies gather loading code to read using
`GatherRow`, `TaxResult`, and `QueryTaxResult`.

4. #2446 removes old,
unused functions that are rendered redundant by the new classes. Also
removes associated tests.

## Additional details for this PR (#2439) 

1. Renamed existing `namedtuple`s. This renaming allows me to introduce
modified `dataclasses` (below) with the same names without breaking
functioning code. This is temporary, as these are removed in #2443.
- `QueryInfo` --> `QueryInf`
- `SummarizedGatherResult` -> `SumGathInf`
- `ClassificationResult` --> `ClassInf`

2. Update these `namedtuple`s to dataclasses to allow additional
functionality
- `QueryInfo`
- `SummarizedGatherResult`
- `ClassificationResult`

These contain several advantages over the prior namedtuples. In
particular, dataclass post-initialization methods allows type checking
and casting string--> float or int as appropriate. I also added methods
to these dataclasses to estimate ANI and produce formatted dictionaries
for each output format. This pulls formatting into one place, rather
than independent output-writing functions.

3. Add dataclasses for reading and manipulating gather results.
- `GatherRow`, for reading a each row from a gather file
- innately checks that all required gather colnames are present when
loading each gather csv
- `TaxResult` for storing a single row from gather file
- get and store `LineageInfo` taxonomic information directly with each
row
    - `get_match_lineage` and `get_ident` are now methods
- tracks whether lineage matching was attempted, including if the ident
was `missed` or intentionally `skipped`. We don't actually allow
skipping from the cli (yet?), but it's enabled in all methods to
preserve existing functionality. I think I was using this to skip
identical/exact database matches.
- `QueryTaxResult` for storing all `TaxResult`s associated with a single
query
  -  only allows summarization over gather results from same query
- more robust/simplified tracking of results with missed & skipped
taxonomic identifiers + perfect matches
- use `LineageInfo` classes (within `TaxResult`) for lineage tracking +
summarization, to get all benefits (optional `taxid`, any hierarchical
ranks, etc)
- contains methods to replicate the summarization previously done within
`summarize_gather_at` in `tax_utils.py` and the classification
thresholding in `genome` within `__main__.py`.

Co-authored-by: C. Titus Brown <titus@idyll.org>
@bluegenes bluegenes merged commit 19374af into use-new-tax-classes Feb 7, 2023
@bluegenes bluegenes deleted the tax-cleanup branch February 7, 2023 20:50
bluegenes added a commit that referenced this pull request Apr 5, 2023
)

## Add taxonomic utilities for LINs; enable and test `tax metagenome`

With taxonomy refactoring (#2437, #2439, #2443, #2446, #2466, #2467), we
are (mostly) no longer tied to named ranks. Here, I add a class for LIN
taxonomies and use it within `tax metagenome` to allow summarization up
LINs and reporting at specified `lingroups`.

With this PR, users can now use the flag `--lins` to read and use `lin`
taxonomies from the provided tax (`-t`, `--taxonomy`) file. If used,
`sourmash tax` will look for a `lin` column in the taxonomy file instead
of looking for `superkingdom`...`strain` columns. The `lin` column
should contain `;`-separated LINs, preferably with a standard number of
positions (e.g. all 20 positions in length or all 10 positions in
length).

For `tax metagenome`:

By default, `tax metagenome` will summarize up _all_ available ranks/LIN
positions. If a `lingroup` file is provided, we will also report a
subset of this summary: just the LIN prefixes that match groups in the
`lingroup` file. The `lingroup` file requires two columns in any order:
`name`, the name of the group, and `lin`, the lin prefix of the group.
The prefix will be used to select results from the full summary for
reporting. The `lingroup` format will build a file with the following
name: `{base}.lingroup.tsv`, where `{base}` is the name provided via the
`-o`,` --output-base` option.

## Demo / Tutorial

A draft tutorial is available
[here](https://sourmash--2469.org.readthedocs.build/en/2469/tutorial-lin-taxonomy.html).
Note that it does not contain the installation info for this branch (see
below). You can run the interactive version via binder
[here](https://mybinder.org/v2/gh/bluegenes/2023-demo-sourmash-LIN/HEAD?labpath=sourmash-lin-demo.ipynb)


## Testing

### Option A: Use the Demo Binder

You can test via the
[binder](https://mybinder.org/v2/gh/bluegenes/2023-demo-sourmash-LIN/HEAD?labpath=sourmash-lin-demo.ipynb).
You can add new cells or modify any existing cells, and even download
additional files for testing. The downside is that you'll have to make
sure to download and save your results, since the binder won't save them
for you.

### Option B: Alternatively, install on your own computer/cluster: 
Here is one way to test this code before it gets fully integrated into
sourmash:
- If you don't have conda, I'd recommend installing `mamba`,
[instructions
here](https://mamba.readthedocs.io/en/latest/installation.html) instead.
- if you do have `mamba`, replace the word `conda` with `mamba` in the
following commands.

Download an environment file that points to this branch:
```
curl -JLO https://raw.githubusercontent.com/bluegenes/2023-demo-sourmash-LIN/main/sourmashLIN.yml
```
Create a virtual environment using this file:
```
conda env create -f sourmashLIN.yml
```
Activate that environment:
```
conda activate smashLIN
```
make sure `--lins` is in the `--help` for `sourmash tax metagenome`:
```
sourmash tax metagenome --help
```



## Command to run

The command to run is this one: 

```
sourmash tax metagenome -g $gather_csv  -t $taxonomy_csv \
                        --lins --lingroup $lingroups_csv
```

## Types of files you'll need

1. sketches of query metagenome
2. sketches of reference genomes (database)
3. taxonomy file with LIN information (two columns required: `ident`,
`lin`)
4. lingroup information file (two columns required: `name`, `lin`) 


To exit the environment when you're done testing, use `conda deactivate`

> Reminder, if you have `mamba`, you can use it in place of `conda` in
the commands above.


example `lingroup` output format. Note that the `1;0`.. paths are always
grouped together, but may come before or after the `0;0` and `2;0`
groups.

```
name	lin	percent_containment	num_bp_contained
lg3	2;0;0	1.56	192000
lg1	0;0;0	5.82	714000
lg2	1;0;0	5.05	620000
lg3	1;0;1	0.65	80000
lg4	1;0;1;0;0;0;0;0;0;0;0;0;0;0;0;0;0;0;0;0	0.65	80000
```

```
name	lin	percent_containment	num_bp_contained
lg2	1;0;0	5.05	620000
lg3	1;0;1	0.65	80000
lg4	1;0;1;0;0;0;0;0;0;0;0;0;0;0;0;0;0;0;0;0	0.65	80000
lg1	0;0;0	5.82	714000
lg3	2;0;0	1.56	192000
```


## A few implementation details:

- In `tax_utils.py`, I add a `LINLineageInfo` class for using and
manipulated LIN taxonomies. It implements new methods to enable
specifically reading in `LIN` taxonomies into the class, but otherwise
uses the taxonomic utilities available in `BaseLineageInfo`, e.g.
taxonomic summarization up ranks, assessing whether two taxonomies are a
match at a given rank.
- In `tax_utils.py`, I add functionality for reading `lingroup`
information and reporting taxonomic summarization specifically at these
ranks.

Changes and Additions:
- [x] Add `LINLineageInfo` for working with `LIN` taxonomies
- [x] Add method for reading `LIN`s into `LineageDB`
- [x] Add methods for reading `LINgroups` and summarizing to these
- [x] Add LineageTree that can use `LineageInfo` to perform
`build_tree`, `find_lca` functions (originally in `lca_utils.py`) and
produce an ordered list of lineage paths
- [x] Add code + tests to use `LIN`s taxonomy in:
  - [x] tax metagenome
  - [x] tax annotate
  - [x] tax summarize
 
The following require additional changes and will be punted to an
issue/separate PR (see #2499):
  - tax genome
  - tax prepare
  - tax grep

---------

Co-authored-by: C. Titus Brown <titus@idyll.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants