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.

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

Reverted about, added headlines #3387

Merged
merged 5 commits into from
Aug 15, 2024
Merged

Conversation

mahrud
Copy link
Member

@mahrud mahrud commented Aug 2, 2024

  • changed makeDocumentTag to strip -- comments
  • reverted previous behavior of about
  • added headlines as a help function

Here's the upshot: you can get links to documentation along with the headlines:

i1 : headlines about firstFunction

o1 = 0 "firstFunction"      -- a silly first function
     1 "firstFunction(ZZ)"  -- a silly first function

i2 : headlines apropos "hilbert"

o2 = 0 "hilbertBasis"       -- computes the Hilbert basis of a Cone
     1 "hilbertFunction"    -- the Hilbert function                
     2 "hilbertPolynomial"  -- compute the Hilbert polynomial      
     3 "hilbertSeries"      -- compute the Hilbert series          

i3 : headlines methods syz

o3 = 0 "syz(GroebnerBasis)"  -- retrieve the syzygy matrix
     1 "syz(Matrix)"         -- compute the syzygy matrix

The output is a Table, but help ZZ and code ZZ still work.

On the other hand, about has been reverted to it's previous format so the entries are syntax highlighted:

i4 : about heft

o4 = {0 => Macaulay2Doc :: heft                }
     {1 => Macaulay2Doc :: heft vectors        }
     {2 => Macaulay2Doc :: heft(Monoid)        }
     {3 => Macaulay2Doc :: heft(PolynomialRing)}
     {4 => Macaulay2Doc :: heft(QuotientRing)  }
     {5 => Macaulay2Doc :: heft(Ring)          }

Hopefully this is a good compromise.

@mahrud mahrud requested a review from pzinn August 2, 2024 16:19
@mahrud
Copy link
Member Author

mahrud commented Aug 2, 2024

Actually, I'm just using netList to tabulate the output, so it's possible that the links are lost in the web interface ... @pzinn is netList outputting an html table on the web interface?

@mahrud mahrud linked an issue Aug 2, 2024 that may be closed by this pull request
@pzinn
Copy link
Contributor

pzinn commented Aug 6, 2024

netList only produces a Net, so yeah, this is definitely a regression on the HTML side.
also, why NumberedVerticalList.AfterPrint = null? this makes it different from any other list, e.g., its ancestor VerticalList.

@pzinn
Copy link
Contributor

pzinn commented Aug 6, 2024

For reference: (links are in orange)

@mahrud
Copy link
Member Author

mahrud commented Aug 6, 2024

netList only produces a Net, so yeah, this is definitely a regression on the HTML side.

I guess I wouldn't be opposed to changing netList to return a TABLE on the web, but it's okay I'll think of an alternative.

also, why NumberedVerticalList.AfterPrint = null? this makes it different from any other list, e.g., its ancestor VerticalList.

We have plenty of types whose afterprint is silenced, usually because either it's obvious (e.g. integers) or annoying when it shows up (e.g. we don't want a random DIV appear after calling help!). I think the latter logic applies here: it's kind of annoying when a random NumberedVerticalList appears after calling methods. I think NumberedVerticalList was created specifically for methods anyway, and now is also used by hooks and oeis, but literally no other mention of it anywhere. Does this make sense?

@mahrud mahrud force-pushed the quickfix/about branch 4 times, most recently from 26f4379 to e30fdd5 Compare August 6, 2024 08:48
@pzinn
Copy link
Contributor

pzinn commented Aug 6, 2024

Before this PR, the only difference between NumberedVerticalList and OL was that the former had an afterprint and the latter not (following the general principle that List types have afterprint but Hypertext types don't). After the PR, there won't be any difference, so might as well retire NumberedVerticalList...

@mahrud
Copy link
Member Author

mahrud commented Aug 6, 2024

The fix was trivial actually. Now it should look like this:
image

following the general principle that List types have afterprint but Hypertext types don't

I don't know what you mean by general principle. I think this was an arbitrary choice, probably mainly because the Hypertext types aren't exported by default. Net and Describe also don't have an afterprint. I think this is just convenience, but if you feel strongly about this I'm happy to revert that commit.

After the PR, there won't be any difference, so might as well retire NumberedVerticalList...

There is a huge difference! The whole point is that debugging tools that accept lists can accept the output of methods:

i1 : locate methods syz

o1 = {0 => (gb.m2:203:47-203:91)    }
     {1 => (matrix2.m2:260:29-263:4)}

i2 : makeDocumentTag methods syz

o2 = {0 => (Macaulay2Doc :: syz(GroebnerBasis))}
     {1 => (Macaulay2Doc :: syz(Matrix))       }

i3 : help methods syz

o3 = syz(GroebnerBasis) -- retrieve the syzygy matrix
     ************************************************
...
     ------------------------------------------------------------------------------------------------------------------------

     syz(Matrix) -- compute the syzygy matrix
     ****************************************
...

Currently implementing these is as simple as:

locate List        := List     => x -> apply(x, locate)

But if methods returned an OL then you'd have to go through and look in each LI item only to realize that sequences have been turned into lists:

i27 : OL methods syz

o27 =   0. syzGroebnerBasis
        1. syzMatrix

i28 : peek OL methods syz

o28 = OL{LI{syz, GroebnerBasis}, LI{syz, Matrix}}

@pzinn
Copy link
Contributor

pzinn commented Aug 6, 2024

OK so headlines looks great now. only one question left: when do we ever need say methods syz instead of headlines methods syz? can't it output what's currently headlines methods syz and then headlines wouldn't be needed?

@mahrud
Copy link
Member Author

mahrud commented Aug 6, 2024

Absolutely not. methods is fundamentally separate from documentation, and is used very frequently in Core. Many, many combinations (like code methods, locate methods, locate makeDocumentTag methods, help methods) are out there and they only work because of the simplicity of methods.

@mahrud
Copy link
Member Author

mahrud commented Aug 6, 2024

I'd say feel free to define headlines Thing to do headlines methods Thing like you suggest, but I think it's best to let the user choose if they want about or apropos or methods and not pick one for them.

@pzinn
Copy link
Contributor

pzinn commented Aug 8, 2024

correction, this was about about, so my point was, who needs about firstFunction when you can have
headlines about firstFunction.

@mahrud
Copy link
Member Author

mahrud commented Aug 8, 2024

I thought about that, but I actually really like how headlines about blah or headlines apropos "foo" sound like plain English. In particular, a feature I really like in certain languages is when you can just learn the basics (e.g. methods, apropos, about, headlines, makeDocumentTag, locate, etc.) then combine them like legos, e.g:

headlines about "hilbert"
locate makeDocumentTag about "hilbert"

and you can switch out about with methods or apropos and you get other useful results. This interoperability makes the language easier to learn (e.g. once they know how headlines about blah works they can apply the same knowledge and try headlines methods blah).

More practically, headlines is a really simple routine, whereas about is pretty complicated and has an option and changes lastabout ... I prefer to keep them separate.

@mahrud
Copy link
Member Author

mahrud commented Aug 14, 2024

@pzinn Does this have your approval now? If so, could you submit a review? (Files Changes Tab > Green Review Changes Button)

@mahrud mahrud merged commit dbfce5a into Macaulay2:development Aug 15, 2024
5 checks passed
@mahrud mahrud deleted the quickfix/about branch August 15, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

help(ZZ) error
2 participants