Skip to content
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

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

Open
maurov opened this issue Apr 22, 2024 · 3 comments
Open

Comments

@maurov
Copy link
Member

maurov commented Apr 22, 2024

@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):

norm_method = self.wids['norm_method'].GetStringSelection().lower()

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
Copy link
Member

@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
Copy link
Member Author

maurov commented Oct 4, 2024

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
Copy link
Member

newville commented Oct 4, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants