Fix custom docstr formatter to work with dataclasses#2494
Fix custom docstr formatter to work with dataclasses#2494haakonvt merged 5 commits intopysdk-release-v8from
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the custom docstring linter was not correctly handling dataclasses, leading to formatting inconsistencies. The changes involve updating the linter's logic to properly parse and format docstrings for dataclasses, and applying the necessary docstring corrections across several data class files to align with the linter's expected format. This ensures consistent and accurate documentation for data classes throughout the codebase. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes a custom docstring linter to correctly handle dataclasses and applies the resulting formatting changes across the codebase. The changes are mostly related to docstring formatting and improving type hints. I've found a logic bug in the linter fix itself that may prevent it from correcting certain invalid docstrings in dataclass __init__ methods. A suggestion to fix this has been provided.
haakonvt
force-pushed
the
fix-custom-docstr-linter-for-dataclasses
branch
from
66cd067 to
74d5d91
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## pysdk-release-v8 #2494 +/- ##
====================================================
+ Coverage 64.59% 64.63% +0.03%
====================================================
Files 467 467
Lines 47729 47729
====================================================
+ Hits 30832 30850 +18
+ Misses 16897 16879 -18
🚀 New features to boost your workflow:
|
haakonvt
changed the title
| with_ (MutableMapping[str, _T_ResultSetExpression]): A dictionary of result set expressions to use in the query. The keys are used to reference the result set expressions in the select and parameters. | ||
| select (MutableMapping[str, _T_Select]): A dictionary of select expressions to use in the query. The keys must match the keys in the with\_ dictionary. The select expressions define which properties to include in the result set. |
There was a problem hiding this comment.
Good question, it does match the base class "directly", but doesn't take into account that we pass
ResultSetExpressionSync for _T_ResultSetExpression, and SelectSync for _T_Select.
I don't think it is worth diving to much into, so I'll add a manual override here instead.
@dataclass
class QueryBase(CogniteResource, ABC, Generic[_T_ResultSetExpression, _T_Select]):
"""Abstract base class for normal queries and sync queries"""
with_: MutableMapping[str, _T_ResultSetExpression]
select: MutableMapping[str, _T_Select]
parameters: Mapping[str, PropertyValue] = field(default_factory=dict)
cursors: Mapping[str, str | None] = field(default_factory=dict)There was a problem hiding this comment.
What's interesting is that class Query(QueryBase["ResultSetExpression", Select]): seems to work as expected... mysterious
There was a problem hiding this comment.
HA! Not so mysterious:
CLS_METHOD_EXCEPTIONS = {
(Query, "__init__"), # Reason: Uses a parameter 'with_'; and we need to escape the underscore
(FunctionHandle, "__init__"), # Reason: Protocol class doesn't have __init__ with return type
}
|
🦄 |
haakonvt
added
risk-review-ongoing
haakonvt
force-pushed
the
fix-custom-docstr-linter-for-dataclasses
branch
from
015c894 to
7239e25
Compare
|
🦄 |
Oh but you do 👨🎤 |
haakonvt
force-pushed
the
fix-custom-docstr-linter-for-dataclasses
branch
from
7239e25 to
181bd57
Compare
haakonvt
force-pushed
the
fix-custom-docstr-linter-for-dataclasses
branch
from
181bd57 to
7d5b8a2
Compare
|
🦄 |
2 participants
While editing signatures lazily awaiting automatic correction by pre-commit, I was surprised to see no changes. I quickly realized our custom docstring formatter was not so fond of
dataclasses😅