-
Notifications
You must be signed in to change notification settings - Fork 162
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
WIP: Constructors #611
base: master
Are you sure you want to change the base?
WIP: Constructors #611
Conversation
Working with people in Bremen on OpenDreamKit's WP 6, I have some patches for constructors as well. I will have a look and coordinate with @stevelinton on this. |
What is the status of this? Do you plan to resume work on this? |
I'll try and catch up with Markus next week and work out a plan |
I think most of the code that I (thought?) I had has been merged in the various type information pull requests I made, and I believe the stuff I thought about is slightly different from what goes under I do remember though that @ssicha, @sebasguts, @frankluebeck, and me had a discussion about constructors (in the GAP sense) when working on MatrixObjs in Aachen, and some documentation might have been produced in the process. For completeness: What we were doing for ODK WP6 was trying to figure out how to produce for any object in a running GAP session how it had been constructed by applying a function (which we called a data constructor) to some arguments. There is some existing code that does this in my github repository that was written by an intern and tries to deduce from the surroundings of a call to |
Yes, I documented the behaviour of constructors in #1391. |
@stevelinton What's the status of this? At the very least it needs a rebase; but it's also unclear to me whether this is something that is planned to be revived one day or not? |
@fingolfin issue #601 is still valid, I think, but the final comment there comes after my work on this PR and essentially says "this isn't as simple as I thought". So I guess the correct thing to do is pick the "obviously correct" bits of this PR, not already taken care of by @sebasguts, if any, rebase them and get them in, and then close this PR but leave the issue open. We should also try and remember to discuss it at the next GAP Days or skype. |
@fingolfin @stevelinton This is currently the only open PR/issue with the label |
I removed that label, as there was a response, although I missed it, and we did not discuss this on either of the two GAP days since @stevelinton's remark... |
I have rebased this PR locally, thus resolving conflicts and reducing it to three commits (the other four were already in master). @stevelinton if you activate the "Allow edits from maintainers" checkbox, then I can also update this PR for you; alternatively, you can get the rebased branch from https://github.com/fingolfin/gap/tree/constructors |
Ticked that box. Thanks |
... and force pushed the rebased PR |
Codecov Report
@@ Coverage Diff @@
## master #611 +/- ##
=========================================
Coverage ? 48.29%
=========================================
Files ? 108
Lines ? 46030
Branches ? 0
=========================================
Hits ? 22229
Misses ? 23801
Partials ? 0
|
77aa046
to
56e5411
Compare
So far this PR fixes the immediate problem reported in #601 that method selection for constructors can miss applicable methods.
There are still a number of things to do in this area:
For instance what kind of group should
AbelianGroupCons(IsGroup,[0])
return?and make the method selection do it.
IS_CONSTUCTOR
(might wait until ObjSets are merged because that would be a perfect solution.AbelianGroup