Sourcery refactored main branch #1
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Comment on lines
-45
to
+53
| use_pr: Optional[PullRequest] = None | ||
| for pr in repo.get_pulls(): | ||
| if pr.head.sha == event.workflow_run.head_commit.id: | ||
| use_pr = pr | ||
| break | ||
| use_pr: Optional[PullRequest] = next( | ||
| ( | ||
| pr | ||
| for pr in repo.get_pulls() | ||
| if pr.head.sha == event.workflow_run.head_commit.id | ||
| ), | ||
| None, | ||
| ) | ||
|
|
Author
There was a problem hiding this comment.
Lines 45-49 refactored with the following changes:
- Use the built-in function
nextinstead of a for-loop (use-next)
Comment on lines
-71
to
+83
| notified = False | ||
| for pr_comment in pr_comments: | ||
| if message in pr_comment.body: | ||
| notified = True | ||
| notified = any(message in pr_comment.body for pr_comment in pr_comments) | ||
| logging.info(f"Docs preview was notified: {notified}") | ||
| if not notified: | ||
| artifact_name = f"docs-zip-{commit}" | ||
| use_artifact: Optional[Artifact] = None | ||
| for artifact in artifacts_response.artifacts: | ||
| if artifact.name == artifact_name: | ||
| use_artifact = artifact | ||
| break | ||
| use_artifact: Optional[Artifact] = next( | ||
| ( | ||
| artifact | ||
| for artifact in artifacts_response.artifacts | ||
| if artifact.name == artifact_name | ||
| ), | ||
| None, | ||
| ) | ||
|
|
Author
| ): | ||
| heroes = session.exec(select(Hero).offset(offset).limit(limit)).all() | ||
| return heroes | ||
| return session.exec(select(Hero).offset(offset).limit(limit)).all() |
Author
There was a problem hiding this comment.
Function read_heroes refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
| with Session(engine) as session: | ||
| heroes = session.exec(select(Hero).offset(offset).limit(limit)).all() | ||
| return heroes | ||
| return session.exec(select(Hero).offset(offset).limit(limit)).all() |
Author
There was a problem hiding this comment.
Function read_heroes refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
| with Session(engine) as session: | ||
| heroes = session.exec(select(Hero).offset(offset).limit(limit)).all() | ||
| return heroes | ||
| return session.exec(select(Hero).offset(offset).limit(limit)).all() |
Author
There was a problem hiding this comment.
Function read_heroes refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
| with Session(engine) as session: | ||
| heroes = session.exec(select(Hero).offset(offset).limit(limit)).all() | ||
| return heroes | ||
| return session.exec(select(Hero).offset(offset).limit(limit)).all() |
Author
There was a problem hiding this comment.
Function read_heroes refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
Comment on lines
-201
to
-208
| relationship_info = RelationshipInfo( | ||
| return RelationshipInfo( | ||
| back_populates=back_populates, | ||
| link_model=link_model, | ||
| sa_relationship=sa_relationship, | ||
| sa_relationship_args=sa_relationship_args, | ||
| sa_relationship_kwargs=sa_relationship_kwargs, | ||
| ) | ||
| return relationship_info |
Author
There was a problem hiding this comment.
Function Relationship refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
Comment on lines
-317
to
+316
| def __init__( | ||
| cls, classname: str, bases: Tuple[type, ...], dict_: Dict[str, Any], **kw: Any | ||
| ) -> None: | ||
| def __init__(self, classname: str, bases: Tuple[type, ...], dict_: Dict[str, Any], **kw: Any) -> None: |
Author
There was a problem hiding this comment.
Function SQLModelMetaclass.__init__ refactored with the following changes:
- The first argument to instance methods should be
self(instance-method-first-arg-name) - Merge dictionary updates via the union operator (
dict-assign-update-to-union)
Comment on lines
-434
to
+431
| foreign_key = getattr(field.field_info, "foreign_key", None) | ||
| if foreign_key: | ||
| if foreign_key := getattr(field.field_info, "foreign_key", None): |
Author
There was a problem hiding this comment.
Function get_column_from_field refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Merge dictionary updates via the union operator (
dict-assign-update-to-union)
Comment on lines
-522
to
+521
| if getattr(self.__config__, "table", False): | ||
| if is_instrumented(self, name): | ||
| set_attribute(self, name, value) | ||
| if getattr(self.__config__, "table", False) and is_instrumented( | ||
| self, name | ||
| ): | ||
| set_attribute(self, name, value) |
Author
There was a problem hiding this comment.
Function SQLModel.__setattr__ refactored with the following changes:
- Merge nested if conditions (
merge-nested-ifs)
Author
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.80%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
Comment on lines
-544
to
+541
| if not getattr(cls.__config__, "table", False): | ||
| # If not table, normal Pydantic code | ||
| m: _TSQLModel = cls.__new__(cls) | ||
| else: | ||
| # If table, create the new instance normally to make SQLAlchemy create | ||
| # the _sa_instance_state attribute | ||
| m = cls() | ||
| m = cls() if getattr(cls.__config__, "table", False) else cls.__new__(cls) |
Author
There was a problem hiding this comment.
Function SQLModel.from_orm refactored with the following changes:
- Swap if/else branches of if expression to remove negation (
swap-if-expression) - Replace if statement with if expression (
assign-if-exp)
This removes the following comments ( why? ):
# the _sa_instance_state attribute
# If table, create the new instance normally to make SQLAlchemy create
# If not table, normal Pydantic code
Comment on lines
-614
to
+605
| if include is None and exclude is None and exclude_unset is False: | ||
| if include is None and exclude is None and not exclude_unset: |
Author
There was a problem hiding this comment.
Function SQLModel._calculate_keys refactored with the following changes:
- Simplify comparison to boolean (
simplify-boolean-comparison) - Replace if statement with if expression (
assign-if-exp)
This removes the following comments ( why? ):
# Updated to not return SQLAlchemy attributes
# recursion, or traversing the whole database
# Original in Pydantic:
# | self.__sqlmodel_relationships__.keys()
# keys = self.__dict__.keys()
# Do not include relationships as that would easily lead to infinite
Comment on lines
-645
to
+628
| def __tablename__(cls) -> str: | ||
| return cls.__name__.lower() | ||
| def __tablename__(self) -> str: | ||
| return self.__name__.lower() |
Author
There was a problem hiding this comment.
Function SQLModel.__tablename__ refactored with the following changes:
- The first argument to instance methods should be
self(instance-method-first-arg-name)
| if not isinstance(query_cache_size, _DefaultPlaceholder): | ||
| current_kwargs["query_cache_size"] = query_cache_size | ||
| current_kwargs.update(kwargs) | ||
| current_kwargs |= kwargs |
Author
There was a problem hiding this comment.
Function create_engine refactored with the following changes:
- Merge dictionary updates via the union operator (
dict-assign-update-to-union)
Comment on lines
-49
to
+53
| if not isinstance(value, uuid.UUID): | ||
| return f"{uuid.UUID(value).int:x}" | ||
| else: | ||
| # hexstring | ||
| return f"{value.int:x}" | ||
| return ( | ||
| f"{value.int:x}" | ||
| if isinstance(value, uuid.UUID) | ||
| else f"{uuid.UUID(value).int:x}" | ||
| ) |
Author
There was a problem hiding this comment.
Function GUID.process_bind_param refactored with the following changes:
- Swap if/else branches of if expression to remove negation (
swap-if-expression) - Replace if statement with if expression (
assign-if-exp)
This removes the following comments ( why? ):
# hexstring
| value2 = Default("bar") | ||
|
|
||
| assert not (value1 == value2) | ||
| assert value1 != value2 |
Author
There was a problem hiding this comment.
Function test_not_equality refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan)
| value2 = "foo" | ||
|
|
||
| assert not (value1 == value2) | ||
| assert value1 != "foo" |
Author
There was a problem hiding this comment.
Function test_not_equality_other refactored with the following changes:
- Inline variable that is only used once (
inline-variable) - Simplify logical expression using De Morgan identities (
de-morgan)
| # Now that this item was checked, remove it from the list | ||
| calls.pop(calls.index(call)) | ||
| assert len(calls) == 0, "The list should only have the expected items" | ||
| assert not calls, "The list should only have the expected items" |
Author
There was a problem hiding this comment.
Function test_tutorial refactored with the following changes:
- Simplify sequence length comparison (
simplify-len-comparison)
| # Now that this item was checked, remove it from the list | ||
| calls.pop(calls.index(call)) | ||
| assert len(calls) == 0, "The list should only have the expected items" | ||
| assert not calls, "The list should only have the expected items" |
Author
There was a problem hiding this comment.
Function test_tutorial refactored with the following changes:
- Simplify sequence length comparison (
simplify-len-comparison)
| # Now that this item was checked, remove it from the list | ||
| calls.pop(calls.index(call)) | ||
| assert len(calls) == 0, "The list should only have the expected items" | ||
| assert not calls, "The list should only have the expected items" |
Author
There was a problem hiding this comment.
Function test_tutorial refactored with the following changes:
- Simplify sequence length comparison (
simplify-len-comparison)
synodriver
pushed a commit
that referenced
this pull request
Jun 24, 2022
perf: raise exception if no matching sqlalchemy type
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Branch
mainrefactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
mainbranch, then run:Help us improve this pull request!