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

Issue adding a group to an Athena project using add_group #547

Open
didudash opened this issue Jan 6, 2025 · 5 comments
Open

Issue adding a group to an Athena project using add_group #547

didudash opened this issue Jan 6, 2025 · 5 comments

Comments

@didudash
Copy link

didudash commented Jan 6, 2025

When adding a group to an Athena project using the add_group method, it runs into an error if:

  • The group does not have data from pre-edge subtraction, and it has for its mu column a name different than mu, for instance mutrans

It fails in the line: 692 of the file: https://github.com/xraypy/xraylarch/blob/master/larch/io/athena_project.py when trying to run the pre_edge method only with the group as attribute, without specifying the arguments for energy and mu.
When the column name is mu it does not fail.

@newville
Copy link
Member

newville commented Jan 6, 2025

@didudash Thanks - that is clear (and sounds believable!). I'll look into that.

I agree that adding any pair of energy/mu arrays should be possible, and this almost certainly needs more testing.

@maurov
Copy link
Member

maurov commented Jan 8, 2025

@didudash thanks for pointing this. but I would rather recommending providing a group with mu attribute, not mutrans. This quickly fixes your issue. If you are interested, I give further details below.

@newville I am reluctant in permitting adding any attribute name, but rather force it. For a XAS group the names for energy and mu should be... energy and mu. If the "mu" array contains a fluorescence, transmission or whatever "mu-like" array, this information should not go into the attribute name, but into another attribute (= metadata) - the "mode". If we permit using any group name for specific arrays that have a very specific meaning, it would be extremely difficult to validate and test. Furthermore, the data model would be inconsistent and each Larch function should perform a tons of tests for the input names at the beginning of each execution. All this would be avoided by adopting, for example, a @dataclass decorator in Group.

This remark goes into the direction of refactoring the Group object, as I was pointing out in #544 too. I think we should specialize the Group object for different type of data. This the direction of standardization we are doing with the XAS metadata working group and the coming NXXas definitions. So, IMHO, if we are dealing with a XAS Group, the object should have at least energy (in eV), mu and mode, where mode is the key of a dictionary well defining what is inside the "mu" array - fluorescence (partial or total), transmission, HERFD, etc.

@newville
Copy link
Member

newville commented Jan 8, 2025

I agree.

I would probably say that if you have a group with mu, look for mutrans or mufluor and use that before giving up.

@didudash
Copy link
Author

I appreciate your answers @newville and @maurov thank you for the detailed explanation. Having then energy, mu and mode should be enough. If I understand mode could be added in a future release?

@newville
Copy link
Member

@didudash @maurov with 2025.1.1 (Just to prove a discussion Mauro and I had about version numbers, 2025.1.0 was messed up for installation/distribution problems), it should be that you can add a group that has "energy" and "mutrans" arrays.

So, I think this is "solved", or at least "better".

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

3 participants