Fix type hinting errors in main_orion.py#85
Open
sonra44 wants to merge 1 commit into
Open
Conversation
Co-authored-by: sonra44 <215552628+sonra44@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts type handling for radar PPI rendering in main_orion.py to satisfy mypy when optional rendering backends are missing or dynamically typed, primarily via assignment ignores and casts around the PPI renderer and textual widget updates. Class diagram for updated PPI renderer typing in main_orion.pyclassDiagram
class MainOrionConsole {
- ppi_renderer : PpiRendererOrNone
_render_radar_ppi() void
_seed_radar_ppi() void
}
class PpiRenderer {
render_tracks(payloads PayloadList) PpiFrame
}
class TextualPpiWidget {
update(content PpiContent) void
}
class PpiRendererOrNone
class PayloadList
class PpiFrame
class PpiContent
class Any
MainOrionConsole --> PpiRendererOrNone : ppi_renderer
PpiRendererOrNone --> PpiRenderer : optional
MainOrionConsole --> TextualPpiWidget : ppi_widget
MainOrionConsole ..> Any : cast_for_ppi_update
TextualPpiWidget ..> Any : dynamically_typed_backend
PpiRenderer --> PpiFrame : returns
PayloadList --> PpiFrame : items
PpiFrame --> PpiContent : displayed_as
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughОбзорДобавлены аннотации типов и приведены в соответствие с типизацией в Изменения
Оценка трудоёмкости проверки кода🎯 2 (Простые) | ⏱️ ~8 минут Стихотворение
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Instead of adding multiple
# type: ignore[assignment]s on_ppi_renderer, consider tightening the declared type (e.g., aUnionor a protocol/interface that matches the different renderer implementations) so mypy can validate assignments without ignores. - For the
cast(Any, ppi)calls, it would be preferable to annotateppiwith the concrete type (or a minimal protocol exposing.update) so that we keep type safety rather than opting out withAny.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of adding multiple `# type: ignore[assignment]`s on `_ppi_renderer`, consider tightening the declared type (e.g., a `Union` or a protocol/interface that matches the different renderer implementations) so mypy can validate assignments without ignores.
- For the `cast(Any, ppi)` calls, it would be preferable to annotate `ppi` with the concrete type (or a minimal protocol exposing `.update`) so that we keep type safety rather than opting out with `Any`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/qiki/services/operator_console/main_orion.py`:
- Around line 1811-1815: Добавьте явный Protocol (напр., PpiRendererProtocol)
описывающий обязательные методы/атрибуты, используемые в рендерере радара
(используйте те же имена методов, что встречаются в коде вокруг
self._ppi_renderer и PpiScopeRenderer), затем замените все cast(Any, ppi) и
присвоения с “# type: ignore[assignment]” на аннотацию типа
Optional[PpiRendererProtocol] (включая атрибут self._ppi_renderer и
параметры/локальные переменные, где ppi используется) и приведите места создания
к соответствующему типу (например, при PpiScopeRenderer(...)). Убедитесь, что
Protocol экспортирован/импортирован там, где нужно, и удалите все локальные #
type: ignore[assignment] в перечисленных местах (строки, где встречались
подавления: места вокруг использования ppi и PpiScopeRenderer).
🪄 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: CHILL
Plan: Pro
Run ID: 388fcc3e-e77f-4c9a-afd2-c2aff8f68377
📒 Files selected for processing (4)
src/qiki/services/operator_console/main_orion.pytests/integration/test_power_soc_dynamics.pytests/integration/test_thermal_core_trip_event.pytests/unit/test_power_load_shedding_order.py
💤 Files with no reviewable changes (3)
- tests/unit/test_power_load_shedding_order.py
- tests/integration/test_thermal_core_trip_event.py
- tests/integration/test_power_soc_dynamics.py
Comment on lines
+1811
to
+1815
| ) # type: ignore[assignment] | ||
| elif PpiScopeRenderer is not None: | ||
| self._ppi_renderer = PpiScopeRenderer(width=ppi_width, height=ppi_height, max_range_m=ppi_max_range) | ||
| self._ppi_renderer = PpiScopeRenderer(width=ppi_width, height=ppi_height, max_range_m=ppi_max_range) # type: ignore[assignment] | ||
| else: | ||
| self._ppi_renderer = None | ||
| self._ppi_renderer = None # type: ignore[assignment] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверка масштаба проблемы и точек применения после рефактора.
# 1) Все места с Any-кастами/assignment-ignore вокруг PPI.
rg -n "cast\(Any,\s*ppi\)\.update|_ppi_renderer\s*=.*type:\s*ignore\[assignment\]" src/qiki/services/operator_console/main_orion.py
# 2) Все обращения к _ppi_renderer (для проверки, что контракт стал единообразным).
rg -n "self\._ppi_renderer" src/qiki/services/operator_console/main_orion.py -C 2Repository: sonra44/QIKI_DTMP
Length of output: 3320
🏁 Script executed:
# Контекст переменной ppi и её объявления/типов
# 1) Ищем, где объявляется/инициализируется переменная ppi в методах, где она используется
rg -B 10 "cast\(Any,\s*ppi\)\.update" src/qiki/services/operator_console/main_orion.py | head -80
# 2) Ищем тип этой переменной через аннотации
rg "ppi\s*[:=]" src/qiki/services/operator_console/main_orion.py | grep -E "ppi\s*:" | head -20Repository: sonra44/QIKI_DTMP
Length of output: 3481
🏁 Script executed:
# Найти определение UI элемента с id "radar-ppi" и его полный тип
rg -n "#radar-ppi|compose|radar_ppi" src/qiki/services/operator_console/main_orion.py | head -30
# Получить полный контекст методов _update_radar_display и _seed_radar_ppi
rg -A 50 "def _update_radar_display\(self\)" src/qiki/services/operator_console/main_orion.py | head -70
# Получить импорты и базовый класс
head -50 src/qiki/services/operator_console/main_orion.pyRepository: sonra44/QIKI_DTMP
Length of output: 3655
🏁 Script executed:
# Найти в compose методах создание виджета `#radar-ppi`
rg -B 5 -A 5 'id="radar-ppi"' src/qiki/services/operator_console/main_orion.py
# Получить полный метод _render_radar_ppi
sed -n '2520,2620p' src/qiki/services/operator_console/main_orion.pyRepository: sonra44/QIKI_DTMP
Length of output: 6932
🏁 Script executed:
# Найти импорты классов рендеров
rg "from.*import.*RadarPpi|from.*import.*RadarBitmap" src/qiki/services/operator_console/main_orion.py
# Найти где определены эти классы
fd -name "*.py" -type f src/qiki/services/operator_console | xargs rg -l "class RadarPpi|class RadarBitmapTGP|class RadarBitmapSixel|class RadarBitmapAuto" 2>/dev/null | head -10Repository: sonra44/QIKI_DTMP
Length of output: 357
🏁 Script executed:
# Получить все импорты в main_orion.py, ищем RadarPpi
rg "RadarPpi|RadarBitmap" src/qiki/services/operator_console/main_orion.py | grep -E "^[0-9]+:(from|import)" | head -20
# Найти все импорты в начале файла
head -100 src/qiki/services/operator_console/main_orion.py | grep -E "from.*import|^import"
# Поиск где эти классы определены или импортируются
rg -n "^from|^import|RadarPpi|RadarBitmapTGP|RadarBitmapSixel|RadarBitmapAuto" src/qiki/services/operator_console/main_orion.py | grep -E "(from|import|Radar)" | head -30Repository: sonra44/QIKI_DTMP
Length of output: 3517
🏁 Script executed:
# Поиск где определяются эти классы (присваивание или условный импорт)
rg -n "RadarPpi\s*=" src/qiki/services/operator_console/main_orion.py | head -10
# Найти все упоминания RadarPpi
rg -n "RadarPpi" src/qiki/services/operator_console/main_orion.py | head -20
# Получить контекст вокруг строк 100-200 (где могут быть условные импорты)
sed -n '100,300p' src/qiki/services/operator_console/main_orion.py | grep -A 3 -B 3 "RadarPpi\|RadarBitmap"Repository: sonra44/QIKI_DTMP
Length of output: 400
🏁 Script executed:
# Найти все четыре класса рендеров
rg -n "class RadarBitmapTGP|class RadarBitmapSixel|class RadarBitmapAuto|RadarBitmapTGP\s*=|RadarBitmapSixel\s*=|RadarBitmapAuto\s*=" src/qiki/services/operator_console/main_orion.py
# Получить определение RadarPpi класса (чтобы увидеть его методы)
sed -n '1190,1250p' src/qiki/services/operator_console/main_orion.pyRepository: sonra44/QIKI_DTMP
Length of output: 3467
🏁 Script executed:
# Проверить есть ли у Static метод update() в Textual
rg -n "def update" src/qiki/services/operator_console/main_orion.py | grep -E "(Static|update.*)" | head -10
# Посмотреть как именно вызывается .update в контексте и на каких параметрах
rg -B 2 -A 2 "\.update\(" src/qiki/services/operator_console/main_orion.py | grep -A 2 "cast(Any, ppi).update" | head -20Repository: sonra44/QIKI_DTMP
Length of output: 1043
Замените cast(Any, ppi) и # type: ignore[assignment] на явный Protocol для рендера радара.
Типизация "чинится" локальными подавлениями в 8+ местах (строки 2562, 2595, 2615, 5955, 5962, 5985, 5992, 6010 и 1811–1815), что размывает гарантии статического анализа в критическом UI-пути. Вместо этого используйте узкий Protocol для ppi объекта и удалите cast(Any, ...) + # type: ignore.
💡 Предлагаемый рефактор
+from typing import Protocol
+
+class _RadarPpiRenderable(Protocol):
+ """Widget с методом update для рендеринга радара."""
+ def update(self, renderable: object) -> None: ...Затем в методе инициализации:
if BraillePpiRenderer is not None:
self._ppi_renderer = BraillePpiRenderer(
width_cells=self._ppi_width_cells,
height_cells=self._ppi_height_cells,
max_range_m=self._ppi_max_range_m,
- ) # type: ignore[assignment]
+ )
elif PpiScopeRenderer is not None:
- self._ppi_renderer = PpiScopeRenderer(width=ppi_width, height=ppi_height, max_range_m=ppi_max_range) # type: ignore[assignment]
+ self._ppi_renderer = PpiScopeRenderer(width=ppi_width, height=ppi_height, max_range_m=ppi_max_range)
else:
- self._ppi_renderer = None # type: ignore[assignment]
+ self._ppi_renderer = NoneИ во всех местах, где используется ppi:
- cast(Any, ppi).update(I18N.bidi("Radar display unavailable", "Экран радара недоступен"))
+ ppi.update(I18N.bidi("Radar display unavailable", "Экран радара недоступен"))- cast(Any, ppi).update(self._ppi_renderer.render_tracks(payloads))
+ ppi.update(self._ppi_renderer.render_tracks(payloads))Это соответствует guideline: src/**/*.py — избегайте Any вне boundaries (JSON/NATS/proto сообщения).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/qiki/services/operator_console/main_orion.py` around lines 1811 - 1815,
Добавьте явный Protocol (напр., PpiRendererProtocol) описывающий обязательные
методы/атрибуты, используемые в рендерере радара (используйте те же имена
методов, что встречаются в коде вокруг self._ppi_renderer и PpiScopeRenderer),
затем замените все cast(Any, ppi) и присвоения с “# type: ignore[assignment]” на
аннотацию типа Optional[PpiRendererProtocol] (включая атрибут self._ppi_renderer
и параметры/локальные переменные, где ppi используется) и приведите места
создания к соответствующему типу (например, при PpiScopeRenderer(...)).
Убедитесь, что Protocol экспортирован/импортирован там, где нужно, и удалите все
локальные # type: ignore[assignment] в перечисленных местах (строки, где
встречались подавления: места вокруг использования ppi и PpiScopeRenderer).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
cast(Any, ...)and# type: ignore[assignment]to fixmypyissues when using dynamic rendering types or when the type is absent due to optional deps handling inmain_orion.py.system_statuspass locally whenrichandtextualdependencies are installed inside the workspace or virtual environment.PR created automatically by Jules for task 5768928180306187665 started by @sonra44
Summary by Sourcery
Enhancements:
Summary by CodeRabbit