Switch to new typing approach#470
Conversation
Use MessageStructureAlias for consistency reasons.
Deprecate python 3.11 as the consequence of new recursive typing approach.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #470 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 103 103
Lines 6084 6074 -10
Branches 773 773
=========================================
- Hits 6084 6074 -10
🚀 New features to boost your workflow:
|
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR modernizes typing across CAN, transport, client, translator, message, and utility APIs; renames several exported aliases; adds explicit exports; updates tests and package metadata; and removes Python 3.11 from CI matrices and classifiers. ChangesTyping modernization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Update CI to no longer use python 3.11
mdabrowski1990
added
the
CI/CD
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Cahnge python version used for docs building.
|
@coderabbitai full review |
|
I'll flag any inconsistencies or missing annotations I find as review comments. ✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
uds/translator/data_record/text_data_record.py (1)
95-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare
__ENCODINGSas aClassVar.Ruff RUF012 flags this mutable class attribute. It is shared configuration rather than instance state, so explicitly declaring it as a
ClassVardocuments the intent and removes the warning.Proposed fix
-from typing import TypedDict +from typing import ClassVar, TypedDict - __ENCODINGS: dict[TextEncoding, _EncodingInfo] = { + __ENCODINGS: ClassVar[dict[TextEncoding, _EncodingInfo]] = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@uds/translator/data_record/text_data_record.py` around lines 95 - 105, Declare the __ENCODINGS class attribute as ClassVar[dict[TextEncoding, _EncodingInfo]] and ensure ClassVar is imported from typing, preserving its existing shared configuration values.Source: Linters/SAST tools
uds/can/addressing/addressing_information.py (1)
23-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate mutable class attribute as
ClassVar.Static analysis flags
ADDRESSING_INFORMATION_MAPPINGas a mutable class-level default withoutClassVar. Since this dict is a shared constant mapping (not meant to vary per instance), annotating it prevents accidental instance-level shadowing and silences the lint warning.♻️ Proposed fix
+from typing import ClassVar + class CanAddressingInformation: """CAN Entity (either server or client) Addressing Information.""" - ADDRESSING_INFORMATION_MAPPING: dict[CanAddressingFormat, type[AbstractCanAddressingInformation]] = { + ADDRESSING_INFORMATION_MAPPING: ClassVar[dict[CanAddressingFormat, type[AbstractCanAddressingInformation]]] = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@uds/can/addressing/addressing_information.py` around lines 23 - 29, Annotate the shared ADDRESSING_INFORMATION_MAPPING dictionary in the addressing information class with ClassVar, importing ClassVar from typing if needed, while preserving its existing key and value types and mapping contents.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/testing.yml:
- Line 125: Add Python 3.14 to the python-version matrix in the static-analysis
workflow so it runs alongside 3.12 and 3.13; if 3.14 cannot yet be supported,
document the exclusion with a link to a tracking issue.
In `@uds/diagnostic_configuration/state.py`:
- Line 31: Replace the redundant None | Any annotations in the state class,
including __current_value and the related declarations around the additional
referenced members, with object | None or the project-specific state value type;
update any corresponding annotations consistently and remove the unused Any
import if applicable.
In `@uds/translator/data_record/abstract_data_record.py`:
- Around line 76-79: Update the MultipleOccurrencesInfo docstring to describe
raw_value and children as tuples rather than lists, matching their annotations
and returned API types.
In `@uds/translator/data_record/conditional_data_record.py`:
- Around line 3-4: Sort the names in the module-level __all__ declaration in
isort-style order to satisfy Ruff RUF022, preserving all existing exports
including DEFAULT_DIAGNOSTIC_MESSAGE_CONTINUATION, MessageStructureAlias,
AbstractConditionalDataRecord, ConditionalMappingDataRecord, and
ConditionalFormulaDataRecord.
- Line 17: Raise the supported Python minimum from 3.11 to 3.12: update
requires-python in pyproject.toml and revise all documentation or version notes
describing Python 3.11+ support to state Python 3.12+ instead.
---
Outside diff comments:
In `@uds/can/addressing/addressing_information.py`:
- Around line 23-29: Annotate the shared ADDRESSING_INFORMATION_MAPPING
dictionary in the addressing information class with ClassVar, importing ClassVar
from typing if needed, while preserving its existing key and value types and
mapping contents.
In `@uds/translator/data_record/text_data_record.py`:
- Around line 95-105: Declare the __ENCODINGS class attribute as
ClassVar[dict[TextEncoding, _EncodingInfo]] and ensure ClassVar is imported from
typing, preserving its existing shared configuration values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 632a7e04-09ab-41ac-8b54-99f8a7a46674
📒 Files selected for processing (53)
.github/workflows/testing.ymlpyproject.tomltests/software_tests/translator/data_record/test_conditional_data_record.pytests/system_tests/abstract_system_tests.pytests/system_tests/can/python_can/python_can.pyuds/__init__.pyuds/addressing/abstract_addressing_information.pyuds/can/addressing/abstract_addressing_information.pyuds/can/addressing/addressing_information.pyuds/can/addressing/extended_addressing.pyuds/can/addressing/mixed_addressing.pyuds/can/addressing/normal_addressing.pyuds/can/frame.pyuds/can/packet/__init__.pyuds/can/packet/abstract_container.pyuds/can/packet/can_packet.pyuds/can/packet/can_packet_record.pyuds/can/packet/consecutive_frame.pyuds/can/packet/first_frame.pyuds/can/packet/flow_control.pyuds/can/packet/single_frame.pyuds/can/segmenter.pyuds/can/transport_interface/common.pyuds/can/transport_interface/python_can.pyuds/client.pyuds/diagnostic_configuration/ecu_configuration.pyuds/diagnostic_configuration/state.pyuds/message/service_identifiers.pyuds/message/uds_message.pyuds/packet/__init__.pyuds/packet/abstract_packet.pyuds/segmentation/abstract_segmenter.pyuds/translator/data_record/__init__.pyuds/translator/data_record/abstract_data_record.pyuds/translator/data_record/conditional_data_record.pyuds/translator/data_record/formula_data_record.pyuds/translator/data_record/mapping_data_record.pyuds/translator/data_record/raw_data_record.pyuds/translator/data_record/text_data_record.pyuds/translator/data_record_definitions/conditional.pyuds/translator/data_record_definitions/did.pyuds/translator/data_record_definitions/formula.pyuds/translator/service.pyuds/translator/translator.pyuds/transport_interface/abstract_transport_interface.pyuds/transport_interface/logger.pyuds/utilities/common_types.pyuds/utilities/constants/did.pyuds/utilities/constants/dtc.pyuds/utilities/constants/other.pyuds/utilities/constants/rid.pyuds/utilities/constants/subfunctions.pyuds/utilities/conversions.py
💤 Files with no reviewable changes (1)
- pyproject.toml
|
@coderabbitai please help with fixing prioblem reported by https://github.com/mdabrowski1990/uds/actions/runs/29127744779/job/86476838324?pr=470 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd prospector
echo "---profile---"
cat tests/prospector_profile.yaml 2>/dev/null || true
echo "---pyproject mypy section---"
sed -n '1,200p' pyproject.toml | grep -n -A20 -i mypy
echo "---workflow static_code_analysis job---"
sed -n '1,200p' .github/workflows/testing.ymlLength of output: 7833 🤖 web_search executed:
Length of output: 14737 🤖 web_search executed:
Length of output: 8069 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---full pyproject.toml---"
cat -n pyproject.tomlLength of output: 5332 🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -i 'mypy' -e ini -e cfg -e toml
fd -i '^mypy\.ini$|^setup\.cfg$'
echo "---search for mypy section in any config---"
rg -n 'tool.mypy|\[mypy\]' --hidden -g '!node_modules'Length of output: 555 🤖 web_search executed:
Length of output: 10905 🤖 web_search executed:
Length of output: 3994 🤖 web_search executed:
Length of output: 7688 🤖 web_search executed:
Length of output: 2354
What's happening: Prospector wraps Prospector's mypy wrapper ( Recommended fix: Have prospector bypass its output-capturing wrapper by passing - name: Execute static code analysis [prospector]
run: |
- prospector --profile tests/prospector_profile.yaml uds
+ prospector --profile tests/prospector_profile.yaml --direct-tool-stdout udsIf this flag changes prospector's console output formatting in a way you don't like, the alternative fallback is to pin Would you like me to kick off a plan to apply this workflow change?
✏️ Learnings added
Failed to handle agent chat message. Please try again. |
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
1 participant
Description
How Has This Been Tested?
Process
I know the process and did my best to follow it