openzim / python-scraperlib

Collection of Python code to re-use across Python-based scrapers
GNU General Public License v3.0
17 stars 16 forks source link

simplify Optional and Union types #150

Closed elfkuzco closed 3 months ago

elfkuzco commented 3 months ago

Rationale

To simplify the expressions for complex Union and Optional combinations using pipe (|) character

Changes

codecov[bot] commented 3 months ago

Codecov Report

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

Project coverage is 100.00%. Comparing base (7948d6e) to head (cfbabf7).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #150 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 32 32 Lines 1399 1393 -6 Branches 240 240 ========================================= - Hits 1399 1393 -6 ```

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

rgaudin commented 3 months ago

scraperlib support python 3.8+ and this is not possible below 3.10 especially without future annotations

rgaudin commented 3 months ago

Ah we already had the future annotations !

elfkuzco commented 3 months ago

Ah we already had the future annotations !

Yes, this PR was meant to supplement #140 by further removing Union and Optional which I believe are more difficult to read and understand than the pipe character which accomplishes the same function in type annotations. For example, the line:

options: ClassVar[dict[str, str | bool | int | None]] = {}

is more simpler and readable than

options: ClassVar[dict[str, Optional[Union[str, bool, int]]]]