Remove or replace unhandled None default for filter curve name

[teutoburg]

In some cases, those Nones were properly handled, that's fine.

In other cases, omitting filter_curve resulted in an unspecific, unhandled error from Path. Requiring that argument gives a much more useful error instead.

In other cases, the filter_curve argument is preceded by other optional argument with meaningful defaults, so using "V" here seems to make sense.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.83%. Comparing base (781f618) to head (0ca15f9).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #118   +/-   ##
=======================================
  Coverage   76.83%   76.83%           
=======================================
  Files           8        8           
  Lines         803      803           
=======================================
  Hits          617      617           
  Misses        186      186           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[hugobuddel]

I'm not sure this change is correct. What does the filter_curve mean when the amplitude is in ABmag? I think I never worked with AB magnitudes, so I don't really know. But I thought that the point of ABmag kinda is that they are (supposedly) filter independent? Or well, independent of a reference, which amounts to the same thing in practice I think.

So if the default of amplitude is in ABmag, then I think it makes sense to have the default of filter_curve to be None. But when the user specifies the amplitude as a Vega magnitude, then a filter_curve must be applied. At least that is how I think it should work, but I could be wrong.

[teutoburg]

I don't know too much about ABmag (or STmag) either, so I'm not sure. In any case, leaving the default of filter_curve=None will always result in an error once we reach Path(filter_curve), regardless of ABmag. So if it turns out that ABmag don't need a filter, we'd need to rewrite that part as well (which is fine, just pointing it out).

[teutoburg]

Both of these functions use a flat_spectrum as a reference. Honestly I'm also not entirely sure how correct that is...

[hugobuddel]

Both of these functions use a flat_spectrum as a reference. Honestly I'm also not entirely sure how correct that is...

I don't know whether that is correct either. But I think it makes sense that ABmagnitudes would still require some kind of reference spectrum (because there must be some kind of prior about the energy distribution); that would mean that if amplitude is in ABmag, then the filter_curve should automatically be set at some point in the code.

I don't know too much about ABmag (or STmag) either, so I'm not sure. In any case, leaving the default of filter_curve=None will always result in an error once we reach Path(filter_curve), regardless of ABmag. So if it turns out that ABmag don't need a filter, we'd need to rewrite that part as well (which is fine, just pointing it out).

Yes, we should understand what is going on. Because having a default that is nonsensical (not sure it is) might lead to hidden problems.

Good luck; I won't stop you from merging this, because I also don't want to put in the time to investigate whether there is a better course of action.