Synchronize gwcs region with changes in jwst/romancal

[WilliamJamieson]

There are copies of region.py in both jwst and romancal. This PR brings the gwcs version in line with these versions, so they can be factored out.

[codecov]

Codecov Report

Attention: Patch coverage is 74.07407% with 7 lines in your changes missing coverage. Please review.

Project coverage is 87.26%. Comparing base (eb9d316) to head (a12edb1).
Report is 18 commits behind head on master.

Files with missing lines	Patch %	Lines
gwcs/region.py	74.07%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #517      +/-   ##
==========================================
- Coverage   87.28%   87.26%   -0.03%     
==========================================
  Files          22       21       -1     
  Lines        3821     3815       -6     
==========================================
- Hits         3335     3329       -6     
  Misses        486      486              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mcara]

For 2x2 arrays _cross may be more efficient than np.linalg.det (less overhead). Same goes for allclose. I would also suggest defining a constant at the top of the module like

_INTERSECT_ATOL = 1e2 * np.finfo(float).eps

instead of recomputing it every time.

[WilliamJamieson]

I you are correct that your "_cross" method is faster at 2.56 ms vs 18.2 ms over 100,000 samples; however, both are faster than np.cross at 54.3 ms over the same input set. If you do want to go with the special method then I think it should be renamed to reflect the fact it is not really a cross product as the cross product does not make sense in 2D, but rather its really just a determinant of the matrix induced by the vectors.

[mcara]

You are correct about _cross in 2D.

[mcara]

same here and also on line 403.

[mcara]

LGTM. Thanks!

[mcara]

Do we really need np.allclose here? I think a simple abs(det) < _INTERSECTION_ATOL is simple and clear enough and without all the numpy array overhead:

In [2]: import numpy as np

In [3]: x = 1.e-12

In [4]: %timeit np.allclose(x, 0, 0, 1e-6)
12.4 μs ± 15.9 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)

In [5]: %timeit abs(x) < 1.e-6
23.6 ns ± 0.0665 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)

[WilliamJamieson]

Do we really need np.allclose here? I think a simple abs(det) < _INTERSECTION_ATOL is simple and clear enough and without all the numpy array overhead:

In [2]: import numpy as np

In [3]: x = 1.e-12

In [4]: %timeit np.allclose(x, 0, 0, 1e-6)
12.4 μs ± 15.9 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)

In [5]: %timeit abs(x) < 1.e-6
23.6 ns ± 0.0665 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)

Sure