afwFilter.h

/**
 * An immutable label for a filter in an exposure or coadd.
 *
 * This class is agnostic to whether the stored label is for an abstract or
 * physical filter.
 * Details about the filter bandpass and transmission curve are stored in a TransmissionCurve object, which
 * can be looked up with the label via `butler.get("transmission_filter")`.
 */
class FilterLabel final : public typehandling::Storable {
public:
  explicit FilterLabel(std::string const &label) _label(label) {}

  /// Allow move and copy: this is a trivial object.
  Filter(Filter const &) = default;
  Filter(Filter &&) noexcept = default;
  Filter &operator=(Filter const &) = default;
  Filter &operator=(Filter &&) noexcept = default;
  ~Filter() noexcept = default;

  /**
   * Create a FilterLabel from a PropertySet (e.g. a FITS header)
   *
   * Reads the "FILTER" header keyword for the label.
   *
   * @param metadata Metadata to process (e.g. a FITS header)
   */
  explicit FilterLabel(
      std::shared_ptr<lsst::daf::base::PropertySet const> metadata);

  std::string const &getLabel() const { return _label; }

  /**
   * Remap special characters, etc. to "_" for database fields.
   *
   * @return The filter label in database-sanatized format.
   */
  std::string const &getDatabaseLabel() const;

  /// Filters compare equal if their strings are equal.
  bool operator==(FilterLabel const &rhs) const noexcept {
    return _label == rhs.name;
  }
  bool operator!=(FilterLabel const &rhs) const noexcept {
    return !(*this == rhs);
  }

  /// Return a hash of this object.
  std::size_t hash_value() const noexcept override;

  bool isPersistable() const noexcept override;

protected:
  // Persistable support
  std::string getPersistenceName() const override;
  std::string getPythonModule() const override;
  void write(OutputArchiveHandle &handle) const override;

private:
  std::string _label;
};


  std::string const &getLabel() { return _label; }

  /**
   * Remap special characters, etc. to "_" for database fields.
   *
   * @return The filter label in database-sanatized format.
   */
  std::string const &getDatabaseLabel();

  /// Filters compare equal if their strings are equal.
  bool operator==(FilterLabel const &rhs) const noexcept {
    return _label == rhs.name;
  }
  bool operator!=(FilterLabel const &rhs) const noexcept {
    return !(*this == rhs);
  }

  /// Return a hash of this object.
  std::size_t hash_value() const noexcept override;

  bool isPersistable() const noexcept override;

protected:
  // Persistable support
  std::string getPersistenceName() const override;
  std::string getPythonModule() const override;
  void write(OutputArchiveHandle &handle) const override;

private:
  std::string _label;
};

This seems either too simple or too complex. Is this intended to be subclassed (since it's not final)? If not, why not just use a string, and have database-sanitizing as a function?

This is to support filter names in ExposureInfo. You're right, it probably should be final. Making it a class instead of a raw string lets us add a little functionality later.

I'm now thinking that getDatabaseName() should be in the python Continue-class though.

We should probably have the same class functionality in both languages wherever possible; otherwise it's confusing. String manipulation doesn't seem like something that needs to be language-specific, even if C++'s string library is pretty clumsy.

That suggestion is to make coding this simpler: it'll be a lot simpler to write it in python, I think, and I'm almost positive we'd never need it in C++. Based on my searching, we essentially never do anything in C++ with the existing afw.image.Filter than newExposure.setFilter(exposure.getFilter()).

I looked into it just now, and it looks like C++'s string handling has improved a lot in C++11; I think all we'd need to do is call std::regex_replace on all non-word characters. 🙂

I think we probably want equality comparison, and of course the tricky part of this is persisting it into Exposure. Whether to enable or disable assignment is also worth thinking about, and that depends largely on whether this is going to be held by shared_ptr (to be like other ExposureInfo components, even if it's a tad wasteful for something this simple). Happy to defer to @kfindeisen on that, as he has a much more up-to-date view of the options and tradeoffs.

I think I'm with Krzysztof on getDatabaseName() being simpler in C++, but I also think that may just reflect a comfort level with C++ that he and I share with each other but not most of the DM team. In any case it is a simple enough question that we could fully code up both possibilities and then ask again.

The current afw Filter is held by ExposureInfo directly, not via shared_ptr. It's never not defined, so I don't see a reason to make it a pointer now.