Skip to content

Panic when adding a node to a child #34

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

Closed
simonpenel opened this issue Jan 12, 2021 · 7 comments
Closed

Panic when adding a node to a child #34

simonpenel opened this issue Jan 12, 2021 · 7 comments
Labels

Comments

@simonpenel
Copy link

Hi, I just start to learn Rust, so I apologize if my question is stupid!
I encounter a problem when building a tree by adding nodes to the root then adding nodes its children:

let mut arbrevide = taxonomy::GeneralTaxonomy::default();
arbrevide.add(0,"Gauche").expect("Impossible d'ajouter un noeud");
arbrevide.add(0,"Droite").expect("Impossible d'ajouter un noeud");
// arbrevide.remove(1).expect("Impossible de supprimer un noeud"); // a remettre
println!("ARBRE VIDE = {:?}",arbrevide);
println!(">>>TAXIDS = {:?}",arbrevide.tax_ids);
println!(">>>LONGUEUR = {}",arbrevide.tax_ids.len());
arbrevide.add(1,"Super").expect("Impossible d'ajouter un noeud");

There is a panic signal when I try to add a node to the node with an internal_taxid equal to 1
Note that if the line
arbrevide.remove(1).expect("Impossible de supprimer un noeud");
is uncommented, it works fine....

I understand that there is a problem at the children_lookup level...

What is the correct way to build a tree then?

thanks for your help

all the best
Simon

@Keats Keats added the bug label Jan 12, 2021
@Keats
Copy link
Contributor

Keats commented Jan 12, 2021

So it looks like we don't actually check if the parent exists before trying to add the new node. This should return an error instead of a out of bound panics. I'll fix it soonish

@simonpenel
Copy link
Author

Thanks a lot!!
( It seems to me that the parent exists in my code? Maybe I missed something)
all the best,
Simon

@Keats
Copy link
Contributor

Keats commented Jan 12, 2021

It's in the children_lookup that we are not inserting it when calling with GeneralTaxonomy::default(). We don't really use that method at all in our code except in one test so I guess it was easy to miss. We mostly load existing ones via https://github.com/onecodex/taxonomy#loading-a-taxonomy

@audy
Copy link
Contributor

audy commented Jun 7, 2022

I ran into this as well:

In [3]: tax = taxonomy.Taxonomy.from_ncbi('ncbi_taxonomy/nodes.dmp', 'ncbi_taxonomy/names.dmp')

In [4]: parent = tax['2']

In [5]: parent
Out[5]: <TaxonomyNode (id=2 rank="superkingdom" name="Bacteria"))>

In [6]: tax.add_node
Out[6]: Taxonomy.add_node  # built-in bound method

In [7]: tax.add_node?
Docstring: Add a new node to the tree at the parent provided.
Type:      builtin_function_or_method

In [8]: tax.add_node('2', '10000000001')

In [9]: tax['10000000001']
Out[9]: <TaxonomyNode (id=10000000001 rank="no rank" name=""))>

In [10]: tax.add_node('2', '10000000002')

In [11]: tax['10000000002']
Out[11]: <TaxonomyNode (id=10000000002 rank="no rank" name=""))>

In [12]: tax.add_node('10000000002', '10000000003')
thread '<unnamed>' panicked at 'index out of bounds: the len is 2423674 but the index is 2423675', src/base.rs:252:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---------------------------------------------------------------------------
PanicException                            Traceback (most recent call last)
<ipython-input-12-248cb6e8e221> in <module>
----> 1 tax.add_node('10000000002', '10000000003')

PanicException: index out of bounds: the len is 2423674 but the index is 2423675

In [13]: tax.children('10000000002')
thread '<unnamed>' panicked at 'index out of bounds: the len is 2423674 but the index is 2423675', src/base.rs:339:9

@Keats
Copy link
Contributor

Keats commented Jun 7, 2022

With #39 ?

@audy
Copy link
Contributor

audy commented Jun 7, 2022

@Keats yep, from fixes:

In [1]: import taxonomy

In [2]: tax =taxonomy.Taxonomy.from_ncbi("ncbi_taxonomy")

In [3]: tax.add_node('2', '10000000001')

In [4]: tax.add_node('10000000001', '10000000002')
thread '<unnamed>' panicked at 'index out of bounds: the len is 2423674 but the index is 2423674', src/base.rs:209:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Keats added a commit that referenced this issue Jun 8, 2022
@Keats
Copy link
Contributor

Keats commented Jun 8, 2022

This should be fixed in the fixes branch

jairideout pushed a commit that referenced this issue Jul 27, 2022
@Keats Keats closed this as completed in adeb4ba Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants