Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-38461

Fix typing support for pex_config fields with optional=True

    XMLWordPrintable

    Details

      Description

      lsst.pex.config.Field definitions with optional=True should be annotated with a type that may be None, or mypy make incorrect complaints about checking whether the field value is None.

       

        Attachments

          Activity

          Hide
          jbosch Jim Bosch added a comment -

          As promised.

          https://github.com/lsst/pex_config/pull/96

          Will start Jenkins a bit later; I suspect main is broken from attempts on a different branch, and I want to chase that down first.

          Show
          jbosch Jim Bosch added a comment - As promised. https://github.com/lsst/pex_config/pull/96 Will start Jenkins a bit later; I suspect main is broken from attempts on a different branch, and I want to chase that down first.
          Hide
          jbosch Jim Bosch added a comment -

          Jenkins says we need changes in dax_apdb, so here they are: https://github.com/lsst/dax_apdb/pull/33

          Would not be surprised to find more of those.

          Show
          jbosch Jim Bosch added a comment - Jenkins says we need changes in dax_apdb, so here they are: https://github.com/lsst/dax_apdb/pull/33 Would not be surprised to find more of those.
          Hide
          jbosch Jim Bosch added a comment -

          I'm bailing out on this and closing the ticket as Won't Fix for now, as the downstream breakage makes it not worth it, even if the problem is real.

          I'm leaving the branch around, but it requires the annotation to be consistent with the value of optional already, and I think we need to instead try to find a way to let

          Field[int]("field a", optional=True)
          

          imply that the field value is int | None rather than int. That will fix existing code rather than forcing existing code to be fixed.

          It's at least a lot harder to do that, and it may be effectively impossible; I think it'd have to involve a lot of typing.overload on Field constructors.

          Show
          jbosch Jim Bosch added a comment - I'm bailing out on this and closing the ticket as Won't Fix for now, as the downstream breakage makes it not worth it, even if the problem is real. I'm leaving the branch around, but it requires the annotation to be consistent with the value of optional already, and I think we need to instead try to find a way to let Field[int]("field a", optional=True) imply that the field value is int | None rather than int . That will fix existing code rather than forcing existing code to be fixed. It's at least a lot harder to do that, and it may be effectively impossible; I think it'd have to involve a lot of typing.overload on Field constructors.

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            jbosch Jim Bosch
            Reviewers:
            Nate Lust
            Watchers:
            Jim Bosch, Nate Lust
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.