-
Notifications
You must be signed in to change notification settings - Fork 120
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
Add @attr
types in simple cases
#4059
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.
Those in my courtyard are fine. ( src/AlgebraicGeometry/Schemes/CoveredSchemes/Objects/Properties.jl )
ToricVarieties is in @HereAround 's courtyard.
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.
I'm not @jankoboehm , but from my point of view the changes around rings are spot-on.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4059 +/- ##
==========================================
- Coverage 84.66% 84.63% -0.04%
==========================================
Files 600 600
Lines 82610 82610
==========================================
- Hits 69944 69919 -25
- Misses 12666 12691 +25
|
For |
Yes, this is o.k. |
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.
As everyone answered in the meantime, I now put this on approve.
* Add `@attr` type in LieAlgebras * Add `@attr` type around lattices * Add `@attr` type around schemes * Add `@attr` type around rings * Fix some inference issue in group code
If one only adds
@attr
to a function without giving a type, one shadows all inference of this function for callers of this function as the@attr
code changes the inference result toAny
. This PR thus adds types there, where it was obvious to me what the return type is, or where the compiler was able to prove a concrete return type (if one removes the@attr
to allow for real inference).The commits are partitioned by the area they touch. Let me ping some people in the different areas here to make them aware / get a review.