DM-43101: Add exposure.can_see_sky metadata field #979
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #979 +/- ##
==========================================
- Coverage 88.92% 88.92% -0.01%
==========================================
Files 329 329
Lines 42349 42351 +2
Branches 8704 8704
==========================================
Hits 37660 37660
- Misses 3439 3441 +2
Partials 1250 1250 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I'd prefer to revert the Union -> | commit and tell Ruff to either ignore that rule (ideal) or ignore at least the majority of those lines, as I think Union is better for anything multi-line (or potentially multi-line in the future).
I'm also not sold on the Self commit, as that is a subtle definition change rather than just notation, when inheritance is in play; I prefer not to use Self unless I want to say that derived classes must return the most-derived type. Looks like we could be using from __future__ import annotations in ButlerLogRecords instead, though - probably dates from when pydantic couldn't handle that.
python/lsst/daf/butler/json.py
Outdated
| @@ -42,7 +44,7 @@ class SupportsSimple(Protocol): | |||
| serialization using "simple" methods names. | |||
| """ | |||
|
|
|||
| _serializedType: Type | |||
| _serializedType: type[BaseModel] | |||
There was a problem hiding this comment.
Should actually by ClassVar[type[BaseModel]], I suspect.
Aside: this is correct for the code as it is now, but I think we really want to support things that are recognized by Pydantic other than BaseModel (like typing.TypedDict) as "simple" objects in the future. I think that would involve making this into:
class SupportsSimple(Protocol, Generic[_T]):
_adapter: ClassVar[pydantic.TypeAdapter[_T]]
There was a problem hiding this comment.
Maybe, but TypeAdapter doesn't support model_dump_json which is what the code relies on (it's called dump_json with TypeAdapter).
I'll do your ClassVar fix.
scons does not (yet) install standard python package metadata so this test can fail if you have safir installed.
After discussion with Jim Bosch we have agreed that multi-line type aliases can include Union for clarity and as a result of that the UP007 warning is now ignored.
This was missing from a previous ticket.
No attempt is made to generate a value when importing from an old universe. (replaces #972)
Checklist
doc/changesconfigs/old_dimensions