search

spacetelescope / poppy

Physical Optics Propagation in Python
https://poppy-optics.readthedocs.io
BSD 3-Clause "New" or "Revised" License
219 stars 72 forks source link

consider deprecating _accept_wavefront_or_meters wrapper in wfe.py #312

Closed mperrin closed 5 years ago

mperrin commented 5 years ago

As discussed in comments for #311, in wfe.py there is a decorator function _accept_wavefront_or_meters which lets you pass either a Wavefront object or a scalar wavelength in meters to the get_opd functions etc. This behavior isn't consistent with the rest of the API, and it doesn't take into account the slight differences between Wavefront and FresnelWavefront classes. Can't remember now what functionality motivated this.

It seems simplest and most elegant to just rip this code out, and have those functions behave consistently with everything in optics.py: the user must supply a wavefront (i.e. instance of a BaseWavefront subclass) as the function input, or else an exception is raised.

mperrin commented 5 years ago

cc @douglase

mperrin commented 5 years ago

I tested, and nothing appears to break if this wrapper is removed (except the two tests in test_wfe.py that specifically use it, and those would just get removed along with it).

Relatedly, the get_opd functions in wfe.py are nonstandard in also having a units argument, to specify that the OPD can be returned in meters or in waves. This is not how any other optic classes work, and should also be stripped out in the name of consistency.

mperrin commented 5 years ago

This behavior has also previously been identified as problematic in #199. Removing it would close that issue.