search

desihub / desimeter

DESI coordinates and transformations
BSD 3-Clause "New" or "Revised" License
2 stars 4 forks source link

Refactor desi_fvc_proc and add option to run directly w/o subprocess #189

Closed dkirkby closed 2 years ago

dkirkby commented 2 years ago

This PR refactors the code to process an FVC image in a backwards compatible way.

The original logic was to have bin/desi_fvc_proc do all the processing inline then implement a library functiondesimeter.processfvc.process_fvc that calls the bin script via a subprocess.

The new logic is to implement the processing steps directly in desimeter.processfvc and then have bin/desi_fvc_proc be a thin wrapper. By default, desimeter.processfvc.process_fvc still uses a subprocess call to run the bin script, but it is now also possible to have desimeter.processfvc.process_fvc invoke the processing steps directly.

The reason for this change is that a subprocess does not necessarily run with the same python environment as its calling context, especially when the current environment is managed by conda or a jupyter kernel. The new (non-default) mode ensures that process_fvc uses the current environment.

coveralls commented 2 years ago

Coverage Status

Coverage remained the same at 73.318% when pulling 5759eb63eec4774735bd2b4444d686e30553e21c on fvcproc into 48179d5bf924c3723083ce9d0f5f35c9c38949da on master.

dkirkby commented 2 years ago

Codacy is still complaining that one function is too complex, but I have already refactored the original function into several smaller functions and it doesn't make sense to go much further.

dkirkby commented 2 years ago

@julienguy @sbailey Can I merge this or do you want to review first?

julienguy commented 2 years ago

Looks good. I am merging.