search

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

Struct2xas 2023.3.x #471

Closed maurov closed 8 months ago

maurov commented 8 months ago

Some improvements and bug fixes on struct2xas.

@newville if we release a patched version for 0.9.72 it may be good to include this now, otherwise I will wait for 0.9.73

newville commented 8 months ago

@maurov This seems okay to me. It seems like the main change is to add the sorting of the sites -- is that correct. Should we just merge?

I think my main concern would be the use of Black styling, which makes it sort of hard to tell what the real changes are.
Not a fan of Black.

maurov commented 8 months ago

@newville

@maurov This seems okay to me. It seems like the main change is to add the sorting of the sites -- is that correct. Should we just merge?

The main change is actually of not using CifParser to instantiate the Pymatgen structure object

            self.struct = Structure.from_file(self.file)
            #self.struct = self.cif.get_structures()[0]  #: NOT WORKING!

We need more testing on the FDMNES side and I would keep this unmerged unless you are planning a release soon.

I think my main concern would be the use of Black styling, which makes it sort of hard to tell what the real changes are. Not a fan of Black.

OK, I understand and I apologize for it. In my opinion, using a code formatter is important to uniform the way we code and it comes from free in most editors. If you do not like black, do you prefer something else?

newville commented 8 months ago

@maurov Oh, that's all OK. I'm not in a rush to push out 0.9.73, mostly because of other commitments over the next week or two. Maybe we can aim for December 1. At the rate we're going, I would be willing to say we generally want releases every 60 to 90 days, but that's sort of a rough target, like if we've gone 90 days without a release, we should probably start thinking that we should get a release out soonish.

For formatting, I think I prefer PEP8 recommendations and being sensible. My feelings on Black are that they sort of made up the existence of formatting arguments and then proposed an absurd set of choices with the goal of "ending the endless discussions". But, there were not endless discussions about formatting until Black imposed its unsolicited opinions, bad choices, and inflexibility-as-an-alleged-feature.

maurov commented 8 months ago

@newville since you are planning to release 0.9.73 soon, I propose to merge these few changes in struct2xas. I was planning to work more on it, but I am fully busy on other tasks for the moment.

newville commented 8 months ago

@maurov Thanks -- I'll merge this and describe it as "continued improvements"!