databricks / cli

Databricks CLI
Other
115 stars 39 forks source link

PythonMutator: support omitempty in PyDABs #1513

Open kanterov opened 1 week ago

kanterov commented 1 week ago

Changes

PyDABs output can omit empty sequences/mappings because we don't track them as optional. There is no semantic difference between empty and missing, which makes omitting correct. CLI detects that we falsely modify input resources by deleting all empty collections.

To handle that, we extend dyn.Override to allow visitors to ignore certain deletes. If we see that an empty sequence or mapping is deleted, we revert such delete.

Tests

Unit tests

codecov-commenter commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 53.96%. Comparing base (e22dd8a) to head (b1d8b59). Report is 161 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1513 +/- ## ========================================== + Coverage 52.25% 53.96% +1.70% ========================================== Files 317 353 +36 Lines 18004 20544 +2540 ========================================== + Hits 9408 11086 +1678 - Misses 7903 8653 +750 - Partials 693 805 +112 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kanterov commented 8 hours ago

@pietern having Optional[list[T]] was not helpful because any Python code (including user-specified) had to handle the possibility of missing values while there is no semantic difference in it. It also made many type signatures unnecessarily complex. So we only left it in "create" method signatures to avoid problems with mutating arguments.

In cases where it matters, we can still use Optional[list[T]] in data classes, and it will keep missing values as-is.

We can fix it in a different way by tracking which lists were not modified or ever specified explicitly.