Fix failing lint checks and update broken typings - #105
Conversation
There was a problem hiding this comment.
Pull request overview
This PR resolves type-checking issues by enhancing type safety throughout the SDK, particularly in request handling and configuration. The changes primarily address mypy lint failures that were affecting CI builds and add Data API support to expand the SDK's functionality.
Key changes include:
- Replaced
is not Nonechecks withisinstance(self.request, dict)throughout request handling code for stricter type safety - Added explicit type annotations and missing type stub imports to resolve mypy errors
- Updated ChangeLog.md to document the Data API additions and lint fixes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| setup.py | Added type annotation for version dictionary and types-setuptools dependency to resolve type checking issues |
| learnosity_sdk/request/init.py | Replaced null checks with isinstance() checks for safer dictionary type handling across multiple service methods |
| ChangeLog.md | Documented Data API additions and mypy error fixes for this release |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Looks good to me 💪. I had to make this fix during the hackathon for the Data API demo. Might be worth fixing here in this PR as well if updating the docs to mention Data API is added to the demos |
jack-oconnor-lrn
left a comment
jack-oconnor-lrn
left a comment
There was a problem hiding this comment.
Agreed to address the signature hashing bug in a separate PR
ddea1f8 to
376ae69
Compare
Codacy flags 'assert' statements since they are stripped under Python's -O optimization flag. The removed asserts existed only to narrow Optional/union types for mypy, not to verify behaviour, so replace them with a small as_dict() helper: a real isinstance() check that raises AssertionError with a useful message on mismatch, and narrows the type for mypy without resorting to typing.cast (which performs no runtime check and can silently hide type errors).
This pull request introduces several improvements and fixes across the codebase, primarily focusing on enhanced type safety, expanded Data API support, and improved CI reliability. The most notable changes include stricter type checks for request handling, the addition of Data API configuration and demo, and fixes for mypy lint errors and missing type imports.
Type Safety and Request Handling Improvements
self.requestinlearnosity_sdk/request/init.pyto useisinstance(self.request, dict)for safer type handling, preventing runtime errors and making the code more robust. [1] [2] [3] [4]Data API Enhancements
CI and Linting Reliability
types-setuptoolsinsetup.pyto resolve type checking issues during testing.Setup Script Improvements
versiondictionary insetup.pyfor better clarity and compatibility with type checkers.Checklist
Feature
Bug
Security
Documentation
ChangeLog.md updated
Tests added
All testsuites passed
make distcompleted successfullyBump package version if release coming