-
Notifications
You must be signed in to change notification settings - Fork 1
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
Enable Quantity
typing with syntax of Quantity[unit]
#95
Conversation
Reviewer's Guide by SourceryThis pull request introduces generics to the Sequence diagram showing type checking with generic QuantitysequenceDiagram
participant Dev as Developer
participant TC as Type Checker
participant Q as Quantity[ms]
Dev->>TC: Define function with Quantity[ms]
TC->>Q: Verify parameter type
Q-->>TC: Type matches
TC-->>Dev: Type check passes
Class diagram showing updated Quantity class with genericsclassDiagram
class Generic~A~ {
}
class Quantity~A~ {
+mantissa
+unit
}
Generic <|-- Quantity
note for Quantity "Now supports generic type parameter A
for type-safe unit specifications"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @chaoming0625 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
def f1(a: u.Quantity[u.ms]) -> u.Quantity[u.mV]: | ||
return a |
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.
issue (testing): Missing assertion in test_quantity_type
The test test_quantity_type
defines functions f1
, f2
, and f3
but doesn't include any assertions to verify their behavior. It should assert that these functions either raise expected type errors or return values of the correct types. For instance, f1
should probably raise a type error because it claims to return u.Quantity[u.mV]
but actually returns u.Quantity[u.ms]
.
This pull request introduces type variables and generics to the
Quantity
class in thebrainunit
module and includes corresponding tests. The changes enhance type safety and flexibility when working with quantities.Code example:
This enhance the readability of the Quantity typing.
Type Enhancements:
brainunit/_base.py
: AddedTypeVar
andGeneric
imports to enable the use of generics in theQuantity
class.brainunit/_base.py
: Introduced a new type variableA
to be used with theQuantity
class.brainunit/_base.py
: Modified theQuantity
class to inherit fromGeneric[A]
, making it a generic class.Testing Enhancements:
brainunit/_base_test.py
: Added a new testtest_quantity_type
to verify the type safety and flexibility of theQuantity
class with different type variables.brainunit/_base_test.py
: Removed unnecessary blank lines in thetest_pickle
function for better code readability.