mperrin
commented
6 years ago 
What else is missing?
From a software-engineering perspective rather than an optics perspective, I would say documentation and testing. In particular, an example optical system in a notebook would be nice to play with (or just look at) for evaluating the ergonomics of the API. Also, since this models a physical system, a test case that, e.g., shows it really does recover the right Zernikes (to some precision) when used as a SH sensor would be a useful sanity check.
How does the structure seem?
See my line comments for things that stood out.
Should it be stored in separate file (as it is here) or just another class within optic.py (or something else?)
Hmm. optics.py is already almost 2000 lines, so I think it should be its own file. The existing optics in POPPY are exposed in the main namespace through judicious use of __all__ and from .modulename import * (see https://github.com/mperrin/poppy/blob/master/poppy/__init__.py#L97-L101 for example) so you'll probably want to add a few lines to ensure poppy.Subapertures is a valid identifier.
(It's possible there's a sensible way to split up optics.py into a package with submodules too, but that would probably be a separate PR.)
Here's my first draft of a Shack-Hartmann simulator. Not a finished product, just getting it out there for feedback early.
I've tried to keep it general and make it a "sub_aperture" class which could feed into an IFS or other instrument that samples a wavefront via a regular grid.
Here's a demo notebook showing sampling a wavefront, generating an array of PSFs and a modal retrieval of the input wavefront (this is my first foray into SH wavefront sensing, so it's not at all optimized but it does seem to recover the right Zernikes): https://gist.github.com/douglase/9f70aaffbd683f3af5edf805847a444e
Things needed before merging:
Things that raise a Not Implemented:
Community questions:
douglase included the following code: https://github.com/mperrin/poppy/pull/185/commits