OpenMS / pyopenms-docs

pyOpenMS readthedocs documentation, additional utilities, addons, scripts, and examples.
https://pyopenms.readthedocs.io
Other
45 stars 55 forks source link

SpectrumSettings.SpectrumType "enum" is not in sync with C++ #294

Closed roman-bushuiev closed 2 years ago

roman-bushuiev commented 2 years ago

I would like to check a type of spectrum (centroid/profile) from the MSSpectrum object. The best option seems to be:

spectrum.getType() == pyopenms.SpectrumSettings.SpectrumType.CENTROID

However pyopenms.SpectrumSettings.SpectrumType "enum" does not have all the elements compared to the C++ reference:

> pyopenms.SpectrumSettings.SpectrumType.UNKNOWN
0
> pyopenms.SpectrumSettings.SpectrumType.CENTROID
AttributeError: type object 'pyopenms.pyopenms_7.__SpectrumType' has no attribute 'CENTROID'
> pyopenms.SpectrumSettings.SpectrumType.PROFILE
AttributeError: type object 'pyopenms.pyopenms_7.__SpectrumType' has no attribute 'PROFILE'
> pyms.SpectrumSettings.SpectrumType.SIZE_OF_SPECTRUMTYPE
3

Is there a better way to check a spectrum type? Of course I could compare spectrum.getType() with integers but then I am not sure that 1 , for example, reflects pyopenms.SpectrumSettings.SpectrumType.CENTROID.

jpfeuffer commented 2 years ago

Thanks for the report. Indeed it was apparently forgotten to sync new enum names with the C++ code: https://github.com/OpenMS/OpenMS/blob/develop/src/pyOpenMS/pxds/SpectrumSettings.pxd#L56

I suggest to always look into the PXD as well (issue for adding links to the docstring is this: https://github.com/OpenMS/autowrap/issues/162). Or the (rather new) API docs, which unfortunately currently struggle with nested classes (https://github.com/OpenMS/autowrap/issues/161)

roman-bushuiev commented 2 years ago

Thanks for the information! So, is there a way to check a spectrum type in the current version? 🙂

jpfeuffer commented 2 years ago

I guess, since the order and the length of the enum did not change, you can use the old enum names: PEAKS and RAWDATA.

Instead of CENTROID and PROFILE.

I think they are converted to integers internally anyway.

roman-bushuiev commented 2 years ago

Ok, thank you for the quick responses.

jpfeuffer commented 2 years ago

should be fixed in the next nightlies and the next release