wfondrie / depthcharge

A deep learning toolkit for mass spectrometry
https://wfondrie.github.io/depthcharge/
Apache License 2.0
59 stars 18 forks source link

Existing index doesn't know whether it's annotated #16

Closed bittremieux closed 2 years ago

bittremieux commented 2 years ago

When trying to re-use an existing HDF5 index, I get the following error in hdf5.py on line 80:

AttributeError: 'AnnotatedSpectrumIndex' object has no attribute 'annotated'

Looking at the code, I don't know when the annotated attribute should be set. In fact, _handle is set to None in line 63, so I'm not fully understanding how this piece of code is supposed to work.

wfondrie commented 2 years ago

Looking at the code, I don't know when the annotated attribute should be set

Apparently I messed up and never set it - or if I do, I've made a terrible design choice that I can't find. 🤦‍♂️

In fact, _handle is set to None in line 63, so I'm not fully understanding how this piece of code is supposed to work.

The index is intended to be used with using the context manager. If you look at __enter__ you'll see that _handle is actually the HDF5 file connection, when used in a with statement.

I'm fully up for design suggestions to improve this though! Does the file format at least make sense?

bittremieux commented 2 years ago

I like the design, being able to use it with a context manager is nice. At first I didn't understand how _handle was set, but I see it now.

So as far as I'm concerned, the only issue is that the annotated attribute needs to be set to fix this bug, the rest of the code works nicely.