-
Notifications
You must be signed in to change notification settings - Fork 5
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
🐛 Fix individual loader import fails #97
🐛 Fix individual loader import fails #97
Conversation
Reviewer's Guide by SourceryThe PR modifies the loader imports to handle optional dependencies gracefully. Instead of importing all loaders unconditionally, XML and YAML loaders are now imported within try-except blocks to prevent import failures when their dependencies are not installed. The all list is also built dynamically based on successful imports. Updated class diagram for loader importsclassDiagram
class Loaders {
+csv
+json
+xml
+yaml
}
note for Loaders "The xml and yaml loaders are now imported conditionally."
class ImportHandler {
+__all__
}
Loaders --|> ImportHandler : dynamically builds __all__ list
note for ImportHandler "__all__ list is built based on successful imports."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
We've reviewed this pull request using the Sourcery rules engine. If you would also like our AI-powered code review then let us know.
pystreamapi/loaders/__init__.py
Outdated
|
||
from pystreamapi.loaders.__csv.__csv_loader import csv | ||
from pystreamapi.loaders.__json.__json_loader import json | ||
from pystreamapi.loaders.__xml.__xml_loader import xml | ||
from pystreamapi.loaders.__yaml.__yaml_loader import yaml | ||
|
||
__all__ = [ | ||
'csv', | ||
'json', | ||
'xml', | ||
'yaml' | ||
] | ||
|
||
__all__.append('csv') |
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.
issue (code-quality): We've found these issues:
- Merge consecutive list appends into a single extend (
merge-list-appends-into-extend
) - Move assignment closer to its usage within a block (
move-assign-in-block
) - Merge extend into list declaration (
merge-list-extend
)
@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.
We've reviewed this pull request using the Sourcery rules engine. If you would also like our AI-powered code review then let us know.
Quality Gate passedIssues Measures |
Summary by Sourcery
Bug Fixes: