-
Notifications
You must be signed in to change notification settings - Fork 76
refactor: Simplified type-conforming logic #2964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
refactor: Simplified type-conforming logic #2964
Conversation
Reviewer's Guide by SourceryThe pull request refactors the Sequence diagram for _conform_primitive_property functionsequenceDiagram
participant Caller
participant _conform_primitive_property
participant _handle_datetime
participant _handle_date
participant _handle_timedelta
participant _handle_time
participant _handle_bytes
participant _handle_numeric
Caller->>_conform_primitive_property: _conform_primitive_property(elem, property_schema)
alt elem is datetime.datetime
_conform_primitive_property->>_handle_datetime: _handle_datetime(elem, property_schema)
_handle_datetime-->>_conform_primitive_property: return converted elem
else elem is datetime.date
_conform_primitive_property->>_handle_date: _handle_date(elem, property_schema)
_handle_date-->>_conform_primitive_property: return converted elem
else elem is datetime.timedelta
_conform_primitive_property->>_handle_timedelta: _handle_timedelta(elem, property_schema)
_handle_timedelta-->>_conform_primitive_property: return converted elem
else elem is datetime.time
_conform_primitive_property->>_handle_time: _handle_time(elem, property_schema)
_handle_time-->>_conform_primitive_property: return converted elem
else elem is bytes
_conform_primitive_property->>_handle_bytes: _handle_bytes(elem, property_schema)
_handle_bytes-->>_conform_primitive_property: return converted elem
else elem is float or decimal.Decimal
_conform_primitive_property->>_handle_numeric: _handle_numeric(elem, property_schema)
_handle_numeric-->>_conform_primitive_property: return converted elem
else elem is boolean
_conform_primitive_property->>_conform_primitive_property: elem != 0
_conform_primitive_property-->>Caller: return converted elem
else
_conform_primitive_property-->>Caller: return elem
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2964 +/- ##
==========================================
+ Coverage 91.50% 91.54% +0.03%
==========================================
Files 62 62
Lines 5311 5311
Branches 684 678 -6
==========================================
+ Hits 4860 4862 +2
+ Misses 318 317 -1
+ Partials 133 132 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #2964 will degrade performances by 21.46%Comparing Summary
Benchmarks breakdown
|
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @edgarrmondragon - I've reviewed your changes - here's some feedback:
Overall Comments:
- The new structure is more readable, but consider if the handler functions could be simplified further, perhaps by sharing common logic.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @edgarrmondragon - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding type hints to the
elem
argument in the handler functions for better clarity. - The
_handle_datetime
function doesn't need theproperty_schema
argument, so you can remove it.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
Refactor the type-conforming logic in the _typing module to improve code structure and readability
Enhancements: