-
Notifications
You must be signed in to change notification settings - Fork 14
Fix failing lint checks and update broken typings #105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix failing lint checks and update broken typings #105
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed to address the signature hashing bug in a separate PR
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