Closed shanosborne closed 6 years ago
Hi @shanosborne, this code looks good. Before merging in a new utility function we should also:
poppy/tests/test_utils.py
Those should all be pretty quick additions. In fact, probably very similar code can be used as an example of how to use it and also as the unit test. We can chat more when we meet later this week.
Hmm, interesting. Your fix fixed the tests in almost all build cases. But it's still failing on the oldest configuration we test (numpy 1.8, astropy 1.3), with a linear algebra error about a singular matrix when fitting the interpolation function. See https://travis-ci.org/mperrin/poppy/jobs/365794975
Now, those are fairly ancient versions of those packages (current is numpy 1.14, and astropy 3.0). So one way to approach this would be simply to increment our minimal required versions of those to something more recent. The test passes fine on numpy 1.11 and 1.12. Looks like they made some improvements in the linear algebra code to make it more robust sometime between those two versions. This is not something I want to have to dig into deeply. But uncovering this sort of compatibility issue is precisely why we run the test suite on a whole bunch of different versions of the core packages.
Added function in poppy/utils.py and named it measure_radius_at_ee