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
Other
127 stars 62 forks source link

Adding features to Group class #442

Closed Ameyanagi closed 1 year ago

Ameyanagi commented 1 year ago

@newville I am working on adding a feature to the Group class that allows iteration and getting attributes in a similar manner as a dictionary.

Please look at the notebook attached for an example of how it helps to work on multiple spectra. examples/Jupyter/Iteration_of_Group.ipynb

I believe this will provide a simple and straightforward way of dealing with multiple spectra. (for example, dealing with Athena project files.)

Since I don't think I have permission to make a new branch, can you add a branch to test these features?

I will also open an issue to discuss how it should be dealt with.

Ameyanagi commented 1 year ago

@newville I implemented access with an index because I thought it would be helpful for those who are familiar with pandas, where the data can be accessed either by index or the key. I find it useful, but it will be a potential cause of bugs, and I agree that it might not be needed.

I personally like using pandas, and sometimes I want to access with an index instead of keys, for example, when I want to plot matplotlib with specific cmap numbers. However, this can be done easily with enumerate, and it might not be necessary. I think it can be removed.

I agree with dropping to_dict() in the future through the deprecation step, but at the current moment, I am not sure how much of the code is used in the whole Larch package.

Thank you for the suggestion, I will delete the index access later.

newville commented 1 year ago

@Ameyanagi Yeah, I understand the desire for a Series-like interface. But we don't even try to define the order of attributes in a Group, or even really assert that there are fixed attributes. I would be very reluctant to say that access to attributes by index is ever going to be reliable.

Even for EXAFS data, we do not say there will be a pile of attributes (energy, norm, pre_edge, k, chi, chir_mag, ...) that might start off as undefined but eventually get filled in. Rather we have a Group -- a basic container class that might have those attributes, and we know what they mean if they do exist.

newville commented 1 year ago

@Ameyanagi Do you think we should merge and then modify or modify before merging?

Ameyanagi commented 1 year ago

@newville Hi Sorry for late reply, I was busy this 2 weeks. I just modified the Groups so that integer indexing is not allowed. I raised an error when trying to index through a integer, to show that integer indexing is explicitly disabled.

The functionality shown in the example notebooks were checked.

newville commented 1 year ago

@Ameyanagi Thanks! I'll happily merge this and will be fascinated to see how long it takes for me to get used to it and then not be able to live without it ;)