for removing that weird global state.

Otherwise the proposal seems somewhat elaborate; for example, what is the difference in behavior between a PhysicalFilter and a PhotometricSystemFilter?

How are transmission curves handled? You mention them in the docstrings but don't provide an API.

we're trying to define the filters for multiple instruments in the same Python process (which I think was always impossible, or at least subject to very surprising and inconsistent behavior)

This came up in some of the early ap_pipe/ap_verify unit tests, where I tried to use a mix of obs_test and obs_decam. Trying to load both packages in the same process invariably raised an exception.

Not to get into a naming debate, but this could be considered a FilterName proposal.  It would be beneficial to give some examples of how these classes would be used, not just constructed, and therefore illustrate their effects on current APIs that use afw.image.Filter.

Thank you so much for writing this up. This looks like a much more coherent system than the current one. Some thoughts:

• I think I want to rename your Filter ABC to FilterBase, so that SimpleFilter can be Filter (I'm trying to channel Meredith Rawls here). But I'm not entirely sure, because I don't know where SimpleFilter would be used in the stack.
• PhysicalFilter maps directly to the gen3 physical_filter Dimension, whereas abstract_filter doesn't have quite as obvious an analog: it's not exactly a PhotometricSystem, but it is sort of a Simple. I realize that refactoring gen3's filters would be a part of this, but sorting out how these relate would be useful.
• The existing aliasing system was used inconsistently in the stack, but I do wonder if we also want some official way to note alternate names for a Physical or PhotometricSystem filter (e.g. "W-S-R+" for "HSC-R"), or if we are better off just standardizing on a single name that we use everywhere, and thus drop those aliases entirely.
• Does the PhysicalFilter.full_name have to map cleanly to the filter name from the raw exposure metadata? In DECam, that is definitely not true: the metadata might have "g DECam SDSS c0001 4720.0 1520.0" for the "DECam-g" PhysicalFilter. We can probably do some remapping in astro_metadata_translator, but this is likely to be a problem for many cameras, since there is little cross-observatory metadata consistency.

For use-cases:

• I believe PhysicalFilter.full_name is what would be used in a butler dataId (e.g. "filter=HSC-I") and what should be stored in the Exposure. The latter is the approach I was trying to normalize to in DM-20763.
• A coadd could be made on either PhysicalFilter.full_name or PhotometricSystemFilter.full_name, and probably nothing else?
• Related to this Community post, what should the refcat filterMap s look like? Should they go from PhysicalFilter.full_name to the refcat filter name, or should we try to have a way to continue to implicitly "share" filter names between refcats and our data?

Krzysztof Findeisen:

what is the difference in behavior between a PhysicalFilter and a PhotometricSystemFilter

A PhysicalFilter is associated with a piece of hardware that defines just one piece of the total throughput of a system, and is used in data products that appear at the lowest levels of the pipelines.  Its transmission curve may very (though probably very little) as a function of time.  A PhotometricSystemFilter does not correspond to any piece of hardware; it represents a theoretical full-system throughput that may be some kind of average or idealization of observations through many time-varying components, and is used in data products that are the final outputs of the pipelines.

How are transmission curves handled? You mention them in the docstrings but don't provide an API.

I was referring to (and am not proposing any changes to) the afw::image::TransmissionCurve API.  Mostly, I am proposing here that those APIs remain distinct, in that a Filter never has a or is a TransmissionCurve.  The way to get a TransmissionCurve given a Filter would generally involve a Butler.get call where one of the (possibly many) data ID keys corresponded to one of the Filter's full_name, but I'm not trying to specify any of the details right now.

Kian-Tat Lim:

Not to get into a naming debate, but this could be considered a FilterName proposal.

A fair point (though I'd probably go with "label" or "identifier" instead of name because these each bundle several names).  I don't have a strong opinion on whether to make that switch, and would love to get input from others here; I could certainly see how making something called Filter just a label class could be confusing for someone expecting a TransmissionCurve.

It would be beneficial to give some examples of how these classes would be used, not just constructed, and therefore illustrate their effects on current APIs that use afw.image.Filter.

I believe the only code that uses the wavelength bounds in the current Filter class is Ian Sullivan's DCR code, which I'd eventually like to see move to using TransmissionCurve instead (I'm not in any hurry there).  I had envisioned that current usage being possible in the new system by subclassing SubdivisionFilter to add whatever attributes Ian finds useful.  That has the advantage (IMO) of not introducing any broader expectation for any wavelength information in the classes I've specified.

All of the multi-band coadd detection and measurement code currently uses the filter from the data ID instead of the Filter from the Exposure, in part because the latter is not reliable (or at least harder to control) in terms of what it returns.  I'd hope that after this proposal is implemented we could switch to using the Filter from the Exposure because those would be consistent with the data ID filter, but I think it's a separate question whether it would be wise to do so.

I believe the current Filter names may also be used as lookup keys in mapping reference catalog filters to the filter of the data being processed.  I know little about that system aside from the fact that it's a mess in need of an overhaul.  I'm hoping we can do that with TransmissionCurve and inferred SEDs (as we've always promised to do even at the SRD level) rather than muddling along further with colorterms.  I would love to get feedback from someone more familiar with the current system (John Parejko, Paul Price, Lauren MacArthur?) on how this proposal would fit in and how difficult it might be to keep it working with these changes without a major overhaul.

John Parejko:

I think I want to rename your Filter ABC to FilterBase, so that SimpleFilter can be Filter (I'm trying to channel Meredith Rawls here). But I'm not entirely sure, because I don't know where SimpleFilter would be used in the stack.

A coadd could be made on either PhysicalFilter.full_name or PhotometricSystemFilter.full_name, and probably nothing else?

Yup, it really does come down to where SimpleFilter is used: I think I'd like to eventually get to the point where SimpleFilter is rarely used in pipeline code (though it may remain heavily used in unit tests), and hence it doesn't deserve the shorter name.  But right now, I think it is the appropriate class to use for all of our current direct and PSF-matched coadds, because they are neither limited to a single PhysicalFilter (in general) nor calibrated to a common photometric system; instead we propagate a TransmissionCurve through coaddition that can be used to put individual measurements on a consistent photometric system (given an SED, which we don't yet have code to infer).

And I'm not sure we will actually be able to get to the point of making coadds where the images themselves use a PhotometricSystemFilter, because trying to apply SED-dependent corrections to pixels instead of measurements is always going to be a bit dodgy (though for DCR-corrected coadds, we have no choice).

So, given all of that, I think that suggests that maybe PhotometricSystemFilter should be removed from the current proposal - it's something we could certainly consider adding later, and I think it's worth thinking about now, but there's no need to add it now, when we don't yet have any code that would make use of it.  And if most of our coadd data products at least stand a good chance of having a SimpleFilter, the rename you propose does sound like a good idea.

PhysicalFilter maps directly to the gen3 physical_filter Dimension, whereas abstract_filter doesn't have quite as obvious an analog: it's not exactly a PhotometricSystem, but it is sort of a Simple. I realize that refactoring gen3's filters would be a part of this, but sorting out how these relate would be useful.

The mapping I had in mind was that Gen3's abstract_filter concept maps to Filter.canonical_name here.  That's not inconsistent with your statement, because a SimpleFilter's canonical_name is the same as its full_name, and I think it captures the broad strokes of how I want the Gen3 filter dimensions to work: I want something like abstract_filter/canonical_name to almost always be available as a default way to relate different kinds of filters (primarily the set of physical filters that map to some vague but generally-agreed-upon SDSS-like broadband filter), while breaking the current system's restriction that this is the only way to relate different kinds of filters.

The existing aliasing system was used inconsistently in the stack, but I do wonder if we also want some official way to note alternate names for a Physical or PhotometricSystem filter (e.g. "W-S-R+" for "HSC-R"), or if we are better off just standardizing on a single name that we use everywhere, and thus drop those aliases entirely.

Does the PhysicalFilter.full_name have to map cleanly to the filter name from the raw exposure metadata?

I think these qustions are related, and it's the need to make sure the Filter knows about the name that's used in the raw exposure metadata that means we do need some kind of alias system.  Maybe we should only add those aliases to PhysicalFilter, then?

I believe PhysicalFilter.full_name is what would be used in a butler dataId (e.g. "filter=HSC-I") and what should be stored in the Exposure. The latter is the approach I was trying to normalize to in DM-20763.

...for raws, and at least most other single-epoch images.  I've already discussed coadds a bit above, but I do intend the full_name to be the data ID value regardless of the filter class.

Related to this Community post, what should the refcat filterMap s look like? Should they go from PhysicalFilter.full_name to the refcat filter name, or should we try to have a way to continue to implicitly "share" filter names between refcats and our data?

I think I'd just like to minimize changes to the refcat filterMaps right now, because I think they're just fundamentally flawed when you start thinking about the data model at this level of detail - I think the gap in the proposal you just made is that we also have code that relies on being able to map the filter for a coadd to a reference catalog, even though "filter for a coadd" is not really a well-defined thing.  I think the easiest interim solution is to just have keys in those maps for both PhysicalFilter.full_name and SimpleFilter/canonical_name values, even if that means some of the values are duplicates and the latter mappings don't mean something as precise as the former.  I'd love to get input from others on how to deal with this, but I don't want to expand the scope of this RFC to solving that problem, too; the pressing (i.g. Gen3 transition-blocking) issue is getting rid of the afw global variables.

I was referring to (and am not proposing any changes to) the afw::image::TransmissionCurve API. Mostly, I am proposing here that those APIs remain distinct, in that a Filter never has a or is a TransmissionCurve.

It sounds like you mean something stronger, that a Filter not only does not contain a TransmissionCurve, but does not refer to one, either. If that's the case, and the relationship between filters and transmission curves is only accessible through the Butler, then the text:

A Filter may be associated with transmission curve information, but never holds it.

A PhysicalFilter is associated with a transmission curve

is confusing and should be removed.

A PhysicalFilter is associated with a piece of hardware that defines just one piece of the total throughput of a system... A PhotometricSystemFilter does not correspond to any piece of hardware

This sounds to me like these classes all behave exactly the same. If a Filter is really just an identifier regardless of role, perhaps we only need a single class for it? This would address John Parejko's concern about names without forcing us to write lots of code in terms of passing and holding a FilterBase.

If a Filter is really just an identifier regardless of role, perhaps we only need a single class for it?

I'm coming around to this idea.  I do think it's important that the identifier have some structure to it - we do have use cases for canonical_name vs. full_name, and I think instrument and system.  So a single class would probably have all of these fields but permit some to be None, but that's still a lot simpler than the (more normalized) polymorphic hierarchy.

Is this RFC waiting on anything further?

So a single class would probably have all of these fields but permit some to be None, but that's still a lot simpler than the (more normalized) polymorphic hierarchy.

I think tagged classes would be a greater evil than a class hierarchy – we already have to work around way too many possibly-null values in our code. Your example had only full_name, so I assumed that everything could be normalized to that form. But it sounds like you're saying that clients do need to be able to distinguish between different kinds of filters.

I think this has converged aside from the question of whether to make this a class hierarchy (the original proposal) or use a single struct that may have some optional attributes.  At this point I think it makes sense to just adopt the RFC and see how both of those approaches fit within the codebase on the implementation ticket, especially as the participants in that conversation have dwindled to just me and Krzysztof Findeisen.

The new filter system will be implemented as described in RFC-730.

Hmm... I could swear I set the resolution to "Duplicate", yet it's been changed to "Done".

I think "Retired" implies "Done"; there's no distinction in resolution in the RFC project.