FABLE-3DXRD / xfab

Other
6 stars 7 forks source link

Fixed broken readline behaviour using parameters module from ImageD11 #8

Closed jadball closed 4 years ago

jadball commented 4 years ago

The readline behaviour was very broken when trying to read the parameters from the par files. I used ImageD11's parameters module to fix this.

lgtm-com[bot] commented 4 years ago

This pull request introduces 5 alerts and fixes 5 when merging 9a1d4d78fce5e41e7bca43a67b1a96e56c82a3d6 into f82aeea2c312508940a2b187ccf027bb4eb8d6a5 - view on LGTM.com

new alerts:

fixed alerts:

jadball commented 4 years ago

Note that this would require ImageD11 as an optional dependency to xfab, so I don't mind if these changes aren't merged. Can always just keep this script in-house for now.

jonwright commented 4 years ago

xfab is already a dependency for ImageD11, so maybe it would be easier to copy the ImageD11 code into xfab and then have ImageD11.parameters say something like "from xfab.parameters import *"?

Beware there are still some stupid bugs in the ImageD11 parameters code to be improved for reading too - I think it doesn't like blank lines etc. Long term this information could be stored in a more professional format that can be tracked to see what was done during analysis (e.g. hdf or NeXus or xml etc).

jadball commented 4 years ago

Not a bad idea. Would it be as simple as copying parameters.py from ImageD11 into xfab, changing all references inside ImageD11 to from xfab import parameters, then rebuilding both? If so, I can do those commits to both ImageD11 and xfab.

jadball commented 4 years ago

Note that other projects like S3DXRDS and TIMELESS would need updating too, so it'd require lots of pull requests...

jadball commented 4 years ago

Oh, never mind, I see what you mean. Have ImageD11.parameters as an alias to xfab.parameters? So the entire contents of the ImageD11.parameters file would just contain: from xfab.parameters import * Then we wouldn't need to update any other code.

jonwright commented 4 years ago

Sounds like it should work. Then within xfab you can remove ImageD11 but people using ImageD11 should not notice anything. Thanks!

lgtm-com[bot] commented 4 years ago

This pull request introduces 5 alerts and fixes 5 when merging d663282e932b75d42b5d0bde5be72fab50a0c72e into f82aeea2c312508940a2b187ccf027bb4eb8d6a5 - view on LGTM.com

new alerts:

fixed alerts:

jadball commented 4 years ago

Ok, it's been moved. xfab is still not completely ImageD11-independent due to some scripts in scripts/ requiring other modules like columnfile and grain etc.