Remove anonymous functions from sheaves #4121
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4121 +/- ##
========================================
Coverage 84.65% 84.66%
========================================
Files 626 627 +1
Lines 84218 84342 +124
========================================
+ Hits 71298 71410 +112
- Misses 12920 12932 +12
|
There was a problem hiding this comment.
I had always been confused by your production_function and restriction_function arguments. This is now a lot clearer (thanks to the needs of serialization efforts).
Overall this looks good, but moving stuff at the same time as making this change was not overly helpful for reviewing.
|
Let me know, wether you are still adding stuff to this or whether you would like a complete review. |
|
Thanks for having a look. Unfortunately, moving the functions was unavoidable. Yes, a full review would be appreciated. Nothing changed about the internal mathematics, though. |
| ### temporary workaround | ||
| produce_object(F::AbsPreSheaf, U::AbsAffineScheme) = production_func(F)(F, U) | ||
| produce_object(F::AbsPreSheaf, U::AbsAffineScheme) = produce_object(underlying_presheaf(F), U) |
There was a problem hiding this comment.
Is the comment about the temporary workaround still valid? If not, please change.
| # production functions for new objects | ||
| is_open_func::Function # To check whether one set is open in the other | ||
| production_func::Union{Function, Nothing} # To produce ℱ(U) for U ⊂ X | ||
| restriction_func::Union{Function, Nothing} # To produce the restriction maps ℱ(U) → ℱ(V) for V ⊂ U ⊂ X open |
There was a problem hiding this comment.
here again: please check whether the comment still reflects the current situation and change, if necessary. We do not want to confuse programmers who take a look at this as a model.
Also fixes the issue pointed out by @simonbrandhorst on #4110 .