Refactoring of FieldOfView and more warnings

[teutoburg]

Refactoring

FieldOfView

• The three .make_... methods were very long, but clearly sectioned. I used that to create several smaller auxiliary functions to split things up a bit. Maybe, with some further refactoring in the future, we might be able to merge some of those. Or subclass FieldOfView 🙃
• In several cases, np.unique was used to get a list of unique elements (reference indices for spectra mostly). This is a textbook usecase of a set, so I used that instead.
• Some tests marked as xfail actually (currently) XPASS, so I included them back in. Those were quite useful in making sure the refactoring here didn't break stuff. I think it would be worth looking into why the others still xfail (as they did before this PR).
• FovVolumeList now inherits from collections.abc.MutableSequence, because that's what it really is, making some nice mixin-methods available.

Generators

I turned some "list-appending-in-loop" functions (mostly in optics.fov_manager_utils (sidenote: I'm not sure where the functions in that module are actually used - I hope it's not something abandoned? 😂 )) into generators, meaning they will not return the instantiated list, but a generator, that can either be turned into a list immediately, or just iterated over when access to the whole list (or repeated access) is not needed. This will change the API of some functions, so I updated some tests accordingly.

De-nesting

Various parts of the codebase have almost a whole function inside an if-clause, which is skipped if the condition is not met. Most of these cases can be simplified by an "early return" approach, i.e. checking for the negative of that condition, and aborting if it's not met, otherwise proceeding as normal (I think it's called a "guard clause" sometimes). There are more of these that I will eventually tackle when working on those classes anyway...

In some other cases, things could be de-nested by other approaches, such as combining a double loop into a single iteration.

Warnings

Motivated by the issues a user was experiencing, I included a few checks for observing a source and warnings if those fail:

• Check if the "PC-matrix" in the header has a determinant of 1.0.
• Check if the array from the source is non-empty (in extract_area_from_imagehdu).
• Check if any fields of the source are inside the FOV.

Misc

No more mutable default arguments

Mutable default arguments (i.e. def foo(bar=[]):) can cause unintended consequences (see e.g. this article) and should be avoided. If I haven't missed any, this should get rid of all of them in this repo (there were only a handful of cases anyway). In all cases, None is now used as the default, and check for that.

Boolean conditions

There were a few cases of if foo is True: or if foo is False:, which I replaced with if foo: or if not foo:, respectively.

log instead of print

Self-explaining...

Renaming

Not really worth mentioning, but for completeness: I renamed some variables for clarity (and to shutup pylint).

Docstring

Along the way, I made some fixes to various docstrings, both in content and to comply with Numpy's style manual.

[codecov]

Codecov Report

Patch coverage: 92.15% and project coverage change: +0.03% 🎉

Comparison is base (cb345d1) 80.20% compared to head (bbe822c) 80.23%.

Additional details and impacted files

@@              Coverage Diff               @@
##           dev_master     #265      +/-   ##
==============================================
+ Coverage       80.20%   80.23%   +0.03%     
==============================================
  Files             143      143              
  Lines           14779    14805      +26     
==============================================
+ Hits            11853    11879      +26     
  Misses           2926     2926              

Files Changed	Coverage Δ
scopesim/optics/fov_utils.py	62.90% <33.33%> (-0.15%)	⬇️
scopesim/optics/image_plane_utils.py	93.41% <80.00%> (-0.32%)	⬇️
scopesim/optics/fov_manager_utils.py	82.77% <87.14%> (-0.95%)	⬇️
scopesim/optics/fov.py	91.72% <93.33%> (+1.18%)	⬆️
scopesim/optics/fov_manager.py	96.29% <94.73%> (+0.67%)	⬆️
...pesim/tests/tests_optics/test_fov_manager_utils.py	76.00% <97.56%> (+1.00%)	⬆️
scopesim/detector/detector_array.py	86.00% <100.00%> (ø)
scopesim/tests/tests_optics/test_FieldOfView.py	87.57% <100.00%> (+0.45%)	⬆️

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

[astronomyk]

Nicely done!