search

Quasars / orange-spectroscopy

Other
51 stars 58 forks source link

Add Phase unwrap preprocessor #727

Closed matyson closed 1 month ago

matyson commented 1 month ago

Add a preprocessor to unwrap the phase of a spectra using numpy's unwrap function. I did not include the proper widget implementation since it was implied from our last meeting that this widget better fits in the Orange SNOM add-on.

When looking this other repo, though, i've noticed that in order to extend the Preprocessor widget from the SNOM add-on i should first add this processing function at the Spectroscopy repo and then import it there, but if i'm wrong please let me know how to proper do this PR.

borondics commented 1 month ago

Thanks for pushing this forward, phase unwrap will be very important for the SNOM package. I see that in the orange-snom repository there is no example on how to register a preprocessor into the orange-spectroscopy/Preprocess Spectra widget. @markotoplak, could you please make an example how to best do this?

Also, we should add both the simple phase unwrap and the possibility to correct the phase through complex number rotations as it is done in NeaReader. These are not the same and we will need both. For now maybe we can simply copy the code with some comments that this need to be changed when NeaReader or pySNOM becomes a dependency or orange-snom.

markotoplak commented 1 month ago

@matyson, I just merged Quasars/orange-snom/pull/3, which shows how to write preprocessors in the SNOM add-on. There, orangecontrib/snom/widgets/preprocessors/registration_example.py indeed imports a preprocessor from spectroscopy, but that was done only for demonstration. It does not matter where the "computation" code is - we could have also defined it in the SNOM addon.

So I think this preprocessor code fits the SNOM add-on better too. I suggest you defined it in file orangecontrib/snom/preprocess/phase_unwrap.py or in something a bit more general if you plan to create some more related preprocessors too.

matyson commented 1 month ago

@markotoplak, okay. So, i will move this PR to the SNOM add-on and follow your suggestions, thanks.

For the tests, should i create a orangecontrib/snom/tests directory too? There is one orangecontrib/snom/widgets/tests but i am not sure if it applies to computation testing as well.

markotoplak commented 1 month ago

Yes, please, create the appropriate test folder.