Skip to content
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.

Sign up for GitHub

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

doc: improve doc of NewConstructor #1391

Merged
merged 3 commits into from
Jun 7, 2017
Merged

doc: improve doc of NewConstructor #1391

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
710e6f6
doc: improve doc of NewConstructor
ssiccha Jun 6, 2017
a9f96a0
doc: also hook up DeclareConstructor
ssiccha Jun 6, 2017
9d1bc2e
doc: add an example to NewConstructor
ssiccha Jun 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion doc/ref/methsel.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,11 @@ plus the number <A>val</A> used in the call of
So the argument <A>val</A> can be used to raise the priority of a method
relative to other methods for <A>opr</A>.
<P/>
Note that from the applicable methods,
Note that for operations which are constructors special rules with respect
to applicability and rank of the corresponding methods apply
(see section <Ref Func="NewConstructor"/>).
<P/>
Note that from the applicable methods
an efficient one shall be selected.
This is a method that needs only little time and storage for the
computations.
Expand Down
49 changes: 41 additions & 8 deletions lib/oper.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ end );
## <Ref Func="InstallMethod"/> must require that the <M>i</M>-th argument
## lies in the filter <A>args-filts</A><M>[i]</M>.
## <P/>
## One can install methods for other arguments tuples via
## One can install methods for other argument tuples via
## <Ref Func="InstallOtherMethod"/>,
## this way it is also possible to install methods for a different number
## of arguments than the length of <A>args-filts</A>.
Expand Down Expand Up @@ -585,19 +585,52 @@ end );
## <Ref Func="NewConstructor"/> returns a constructor <A>cons</A> with name
## <A>name</A>.
## The list <A>args-filts</A> describes requirements about the arguments
## of <A>constr</A>, namely the number of arguments must be equal to the length
## of <A>args-filts</A>, and the <M>i</M>-th argument must lie in the filter
## <A>args-filts</A><M>[i]</M>. Additionally a constructor expects the first
## argument to be a filter to then select a method to construct an object.
## of <A>cons</A>. Namely the number of arguments must be equal to the length
## of <A>args-filts</A>, and the <M>i</M>-th argument
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this early wrapping? That causes an unnecessary diff (thus making it a little bit harder to review this PR), and I see no good reason for doing it?

## must lie in the filter <A>args-filts</A><M>[i]</M> for <M>i \neq 1</M>.
## A constructor expects the first argument to be a <E>filter</E> instead
## of an object and it must be a subset of the filter
## <A>args-filts</A><M>[1]</M>.
## <P/>
## Each method that is installed for <A>cons</A> via
## <Ref Func="InstallMethod"/> must require that the <M>i</M>-th argument
## lies in the filter <A>args-filts</A><M>[i]</M>.
## <Ref Func="InstallMethod"/> must require that
## the <M>i</M>-th argument lies in the filter <A>args-filts</A><M>[i]</M>
## for <M>i \neq 1</M>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, weird re-wrapping -- now I have to carefully check what you really changed :-(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I just thought it would look cleaner in the end. Didn't think about the weird-looking diffs it would introduce. I'll keep that in mind in the future, thanks. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Luckily I can use http://www.jefftk.com/icdiff on my local computer to make it better, but the easier it is to already review in the browser, the faster you'll get meaningful feedback

## Its first argument is a filter and must be a subset of the filter
## <A>args-filts</A><M>[1]</M>.
## <P/>
## One can install methods for other arguments tuples via
## One can install methods for other argument tuples via
## <Ref Func="InstallOtherMethod"/>,
## this way it is also possible to install methods for a different number
## of arguments than the length of <A>args-filts</A>.
## <P/>
## Note that the method selection for constructors works slightly differently
## than for usual operations.
## As stated above, applicabilty to the first argument in an argument tuple
## is tested by determining whether the argument-filter is a <E>subset</E> of
## <A>args-filts</A><M>[1]</M>.
## <P/>
## The rank of a method installed for a constructor is determined solely by
## <A>args-filts</A><M>[1]</M> of the method.
## Instead of taking the sum of the ranks of filters involved in its
## <A>args-filts</A><M>[1]</M>, the sum of <M>-1</M> times these values
## is taken.
## The result is added to the number <A>val</A> used in the call of
## <Ref Func="InstallMethod"/>.
## <P/>
## This has the following effects on the method selection for constructors.
## If <A>cons</A> is called with an argument tuple whose first argument is
## the filter <A>filt</A>, any method whose first argument is
## <E>more</E> specific than <A>filt</A> is applicable
## (if its other <A>args-filts</A> also match).
## Then the method with the <Q>most general</Q> filter <A>args-filts</A><M>[1]</M>
## is chosen, since the rank is computed by taking <M>-1</M> times the ranks
## of the involved filters.
## <P/>
## Imagine wanting to construct a new IsMatrixObject without caring
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Imagine" ? So, is this an example? Then I'd start with "For example" to make that clear.

Also, don't you mean IsMatrixObj?

## explicitly in which representation it is created.
## &GAP; then chooses a filter whis is <Q>as close</Q> to
## IsMatrixObject as possible.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This leaves me with more questions. E.g. why wouldn't I just use IsMatrixObj? Why would I only want to be "as close" as possible?
How about giving an actual example? Or perhaps declare a constructor, then install 2-3 methods, and document which of them is used for various (also given) example invocations?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "close" is slightly the wrong word. Method selection in this case will choose a constructor which guarantees to produce an object in IsMatrixObj. All other things being equal it will choose the one that promises the fewest extra filters on the grounds that that is presumably easiest to do.

Copy link
Contributor Author

@ssiccha ssiccha Jun 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll get on that.

## </Description>
## </ManSection>
## <#/GAPDoc>
Expand Down