-
Notifications
You must be signed in to change notification settings - Fork 259
Adds Algebra.Morphism.Construct.DirectProduct #2715
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
Adds Algebra.Morphism.Construct.DirectProduct #2715
Conversation
JacquesCarette
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.
looks good to me
|
Hmm: some things to maybe (re) consider
Suggested refactoring:
|
MatthewDaggitt
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.
And I also agree with @jamesmckinna's suggestion that the actual morphism should be listed explicitly.
jamesmckinna
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.
- level polymorphism
- suggest refactor in the light of #2720
38c20f8 to
1b7d503
Compare
|
I've tried to address the comments above. However, I can't understand how the suggestion in:
would work. By this I mean, that the argument I already rebased this branch on top of (a squashed version of) |
|
Thanks @carlostome yes I think you're right that my suggestion about restructuring the module organisation was wrong. |
|
Now that #2720 has been merged, suggest we try to tie this one up for v2.3 also? |
0b14e5b to
8ea743c
Compare
jamesmckinna
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.
Apart form the final nitpick, looks good to go.
Many thanks for the contribution, and your patience during the review process!
jamesmckinna
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.
Yup, all looks good to me now!
|
So we need an approve from @MatthewDaggitt . |
This PR adds proofs that projections from the product type are structure preserving maps (morphisms) from the direct product construction.
This not complete whatsoever, but at least it gives an initial template that can be extended over time.