xraypy / xraylarch

Larch: Applications and Python Library for Data Analysis of X-ray Absorption Spectroscopy (XAS, XANES, XAFS, EXAFS), X-ray Fluorescence (XRF) Spectroscopy and Imaging, and more.
https://xraypy.github.io/xraylarch
MIT License
138 stars 68 forks source link

[larix] bug in plotting selected groups with mixed normalization methods #511

Open maurov opened 7 months ago

maurov commented 7 months ago

@newville a colleague found a bug in Larix when plotting selected groups which have different types of normalization (e.g. polynomial and mback). The bug comes from (and following):

https://github.com/xraypy/xraylarch/blob/ae6ed7961ecf84a16bd5611fb9c5cc1cc5cecde6/larch/wxxas/xasnorm_panel.py#L587

If fact, the current behavior in Larix when plotting normalized selected groups depends on the current clicked group, that may be a group out of the selected ones. In my opinion, this is a bug.

My expected behavior would be that a normalization function, whatever method is used (standard, mback, area, whatever) should write the norm array of the group, while the plotting function should only plot this attribute, without repeating the normalization itself or using different array names. What do you think?

newville commented 7 months ago

@maurov I agree: this is sort of messy. It is nice to have multiple normalization options, so maybe have norm_poly and norm_mback available for comparison. But norm should be the selected one choice. I'll check this out.

maurov commented 1 month ago

This is somehow related to #531. In fact, the current implementation done to fix #526, that is, having a norm_mback attribute and force this normalization all the time is not the best solution, in my opinion. In my view, the norm attribute should be unique, whatever the method is used to normalize the data. In this way one can plot/use normalized spectra even if they are normalized by different methods. I would propose to add the norm_method attribute and use @property decorator in the Group object to return the correct one. To show my idea, this could be something like:

@property
def norm(self):
    if self.norm_method == "mback":
        return self.norm_mback
    if ...

    return self._norm
newville commented 1 month ago

@maurov Sounds good to me, though I might change self._norm to self.norm_poly