search

HEXRD / hexrd

A cross-platform, open-source library for the analysis of X-ray diffraction data.
Other
56 stars 25 forks source link

Some methods defined in both transforms and rotations.py #692

Closed kevindlewis23 closed 1 month ago

kevindlewis23 commented 1 month ago

There are some functions in transforms (hexrd/transforms/xf.py and hexrd/transforms/xfcapi.py) like mapAngle, rowNorm, columnNorm, angularDistance, quatProductMatrix, unitVector, and arccosSafe that are also defined in rotations.py. Do people use these methods from both places?

Either this should only be in one place, or the implementation in one place should just be an alias of the other. If the second option is better, where should the main implementation live?

kevindlewis23 commented 1 month ago

Also toFundamentalRegion, quatOfLaueGroup, and ltypeOfLaueGroup are in both material/symmetry.py and rotations.py, with the exact same implementations

kevindlewis23 commented 1 month ago

Also material/symbols.py and material/spacegroup.py have the same 1000+ line _hallStr, and then _buildDict which builds a dictionary from the string.

psavery commented 1 month ago

I'd say that when there are duplicated functions, we can definitely eliminate one of the duplicated functions. If we are concerned that users may be importing the duplicated function from the module where it was deleted, it can still be imported within that module to maintain backward compatibility.

kevindlewis23 commented 1 month ago

One of the biggest offenders seems to be the hexrd/wppf directory. peakfunctions.py is unsurprisingly very similar to hexrd/fitting/peakfunctions.py. There are some functions that are exactly the same, and there are some that are the exact same but then one implementation normalizes the output. hexrd/wppf/phase.py has a bunch of classes that share a bunch of methods and exact implementations, which could easily be replaced by extending a base class.