fix: resolve issues #117 and #105 (Codecov + rescale diagnostics) - #120
fix: resolve issues #117 and #105 (Codecov + rescale diagnostics)#120t-uda wants to merge 2 commits into
Conversation
feat(ellcloud): add RescaleDiagnostics and return_diagnostics to rescale() (#105) - Upgrade codecov/codecov-action from v4.0.1 to v5; add continue-on-error: true so a Codecov network glitch cannot fail an otherwise-passing CI run. - Add frozen dataclass RescaleDiagnostics (scale, pre_summary, post_summary) exported from ellphi and ellcloud. - rescale() gains an opt-in return_diagnostics=False flag; default path is fully backward-compatible. - Update ellcloud.pyi with @overload stubs for both call signatures. - Add three new regression tests covering type, shape, pre/post relationship, and consistency with the default return path. https://claude.ai/code/session_01S8zuHdfhWRruSejHtQZo6L
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e2c8315f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @overload | ||
| def rescale( | ||
| self, *, method: str = ..., return_diagnostics: Literal[False] = ... | ||
| ) -> float: ... |
There was a problem hiding this comment.
The new stub defines rescale only for return_diagnostics: Literal[False] and Literal[True], so a common call pattern like flag: bool followed by cloud.rescale(return_diagnostics=flag) is rejected by type checkers even though it works at runtime. This creates a typing regression for users who pass a non-literal bool; add a third overload (or implementation signature) that accepts bool and returns float | RescaleDiagnostics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7a9da13. Added a third bool fallback overload returning float | RescaleDiagnostics so dynamic bool variables are accepted by type checkers. mypy and stubtest both pass.
Generated by Claude Code
Addresses Codex review: a non-literal bool variable passed as return_diagnostics was rejected by type checkers. Add a third overload accepting bool -> float | RescaleDiagnostics as the fallback case. https://claude.ai/code/session_01S8zuHdfhWRruSejHtQZo6L
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Closing this PR in favour of #119 as part of the July 2026 backlog triage. Rationale:
No code is lost: the RescaleDiagnostics implementation lands via #119 (resolves #105). |
1 participant
Summary
Resolves #117 and #105.
Changes
Issue #117 — CI: Codecov upload step can fail even when tests pass
codecov/codecov-actionfromv4.0.1tov5continue-on-error: trueto prevent Codecov network glitches from failing CIIssue #105 — feat: add diagnostics for EllipseCloud.rescale()
RescaleDiagnosticsfrozen dataclass withscale,pre_summary,post_summaryrescale()with opt-inreturn_diagnostics=Falseflag (fully backward-compatible)ellcloud.pyiwith@overloadstubs for both signaturesRescaleDiagnosticsfromellphipublic APITest Results
Remaining Task
.github/workflows/python-app.ymlrequires update for Codecov v5 changeworkflowscope (required by GitHub for.github/workflows/edits)workflowscope to the token and retryuses: codecov/codecov-action@v4.0.1touses: codecov/codecov-action@v5and addcontinue-on-error: trueon line 65All code and test changes are in place and passing. Only the workflow file update is blocked by token permissions.