flatlamp gets spectrum parameter; remove wrapper

[oczoske]

The primary change is the addition of a parameter spectrum to flatlamp(), allowing to set the lamp spectrum directly, thus avoiding constructions like the one that caused AstarVienna/ScopeSim#565.

To allow accessing this parameter (and also fraction and others) I have removed (commented out) the wrapper in micado/__init__.py. This was discussed in #123. To be honest I don't quite follow the reasoning there, but for the function to be useful the user must be able to see and set the parameters explicitely.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.22%. Comparing base (d4c7da2) to head (cbaede5).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #124   +/-   ##
=======================================
  Coverage   74.22%   74.22%           
=======================================
  Files          30       30           
  Lines        1482     1482           
=======================================
  Hits         1100     1100           
  Misses        382      382           

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

[hugobuddel]

The idea of the micado directory is that these sources are MICADO-specific, and MICADO has only one specific flat lamp right? I'm not actually sure how the flat lamps work in MICADO, but they won't have a spectrum that can be freely changed.

So maybe it would make more sense to have a generic flatlamp source in the calibration directory, and then have a specific one in micado?

By the way, I noticed that there are already 2 flat-related functions in calibration.py: lamp() and flat_field(). So I think it is becoming a bit messy.

FWIW, I still think it is worthwhile to keep those top level functions parameterless, but I'm just noting that for completeness sake as I already said that you can change hat if you prefer. But it seems it is still possible to call this one without parameters, so it should be fine. Perhaps then just remove the one you commented out?

The code is otherwise fine.

[oczoske]

This is purely because people are using this function, and they're having problems with it. It is by no means meant to be a realistic representation of the MICADO calibration unit.

[hugobuddel]

Then wouldn't it be better to move the function to the calibration directory?

It is not representative of MICADO, it is used also for other instruments, even by ourselves in the METIS notebooks, to the point that we need to generalize it to accept a different spectrum.

[hugobuddel]

I bring this up now, because it is one thing to have a bad flat lamp representation in the MICADO directory, but it is another thing to modify it so that it can be a less bad representation for METIS, while keeping it in the MICADO directory.

[oczoske]

Let it be then.

[hugobuddel]

For the record: I did approve the PR, in case that was not clear

[hugobuddel]

I approved the PR, because this PR would make things better in all ways.

These PRs are also an opportunity to look a bit further than what is in front of our nose, to foster a shared vision, hence my more elaborate comments. Those comments were in addition to approving the PR as-is.

So feel free to reopen and merge this.

[teutoburg]

By the way, I noticed that there are already 2 flat-related functions in calibration.py: lamp() and flat_field(). So I think it is becoming a bit messy.

That is quite symptomatic of ScopeSim_Templates generally 🙃