Print `methods(f)` with color like stack traces #40251

mcabbott · 2021-03-29T01:12:58Z

Should we re-use something like the new error printing for method lists?

Perhaps the module which(f) should be mostly omitted, to highlight only other-module methods?

(Edit -- deleted an example. The one above does not yet have types in bold, added below.)

Closes #40913 Closes #30110 Closes #36129

thchr · 2021-03-29T03:37:47Z

This is no doubt subjective, but when I'm calling methods what I'm most interested in disambiguating is the different type signatures. This seems different from what one is interested in emphasizing for stack traces (e.g. different modules and path locations) and what is most clearly highlighted with this coloring/highlighting.

mcabbott · 2021-03-29T13:18:38Z

Agree the purpose is different. The hope would be that light grey paths could act as visual space between the methods. And that module colours would flag methods dealing with types they own -- like StaticArrays & BenchmarkTools here:

fingolfin · 2021-03-30T09:14:26Z

I really like this. However, I feel that the colored package names in your examples stand out most -- they are the first thing I see/read when looking at those images. I feel that's reverse from how it should be. Perhaps one can experiment a bit more with this. Either by deemphasizing the package names, or by emphasizing the method argument types more. Besides colors, one could also experiment with e.g. using bold face -- e.g. how does it look if the argument types are printed in bold?

mcabbott · 2021-03-30T15:50:38Z

More subtle colours would be better, but hard to make things precise. Bold seems to vary from doing nothing to feeling like shouting, here's a very crude attempt:

With more fiddling the colour could be moved to apply to the types owned by that package, not the module name. Or (simpler) to highlight the package name in the path, rather than alone.

fingolfin · 2021-03-30T17:54:18Z

To me your last screenshot is by far the best in terms if being able to quickly pick out which methods are there and in particular finding any that I care about

mcabbott · 2021-03-30T19:27:43Z

A slightly more refined version, with only the outermost type bold. This avoids printing 4 lines of bold text at [12], perhaps other things like a::T, b::T (last line) are odd?

Edit -- This also affects the printing of ambiguities, but not of no-method errors:

StefanKarpinski · 2021-03-31T01:47:57Z

Really like these! Seems ok if the coloring and emphasis of stack traces and methods lists are a bit different, but ideal to share the common printing logic aside from those details.

fingolfin · 2021-03-31T05:13:45Z

Nice!

Any idea why in the ambiguity error the first Any is not bold?

thchr · 2021-03-31T18:05:49Z

That is nice!
I do still think, subjectively, that the module names are the most visibly distinct feature in the printout, but it's a lot easier to disambiguate now (and definitely a ton clearer than the current styling).

I wonder if the module name coloring is helpful at all in this context though. To me, right now, they remain the most visible feature in the list, drawing my eye to them as a kind of semi-noisy "headers" of sorts (and notably, the headers come after the content - this is confuses me in the stack trace printing as well a bit; I'm always finding a "header", then I need to remind myself to look upwards to see its associated content).

If the modules were all colored gray, the "where-is-this-method-actually-from" line would serve as a nice and consistent "quasi-whitespace" of sorts between method signatures.
Just wondering out loud here, obviously - the proposed styling is already very nice..!

mcabbott · 2021-03-31T19:51:53Z

Any idea why in the ambiguity error the first Any is not bold?

Probably a bad decision! In looking at examples with a few types & many ::Anys, I was trying to reduce their importance.

I wonder if the module name coloring is helpful at all in this context though.

The hope is that it's a proxy for the package for whose types a method is being added. How often things will end up monochrome or looking like a rainbow in actual use, will depend... I guess colours will be most useful when there are just a few "foreign" methods, which are either the ones you care about, or the ones you can ignore.

and notably, the headers come after the content - this is confuses me in the stack trace printing as well a bit

Agreed. There were other proposals in the giant stack trace threads, like highlighting the [10] or the function name, but (IIRC) these felt like visual puzzles to see what the colours went with. Qualifying the function names didn't look great, and moved further from how @warn etc. print locations.

Here, I worry that highlighting Type{GitCredentialHelper} would similarly introduce an annoying puzzle to solve. Whereas everyone is used to reading the stack traces (or soon will be!)

JeffBezanson · 2021-05-24T21:16:51Z

ideal to share the common printing logic aside from those details.

This!

mcabbott · 2021-05-24T22:08:59Z

It would also be nice to unify the code which prints "MethodError: no method matching" with everything else --this PR doesn't change that, but affects "MethodError: ... ambiguous" because that is shared with methods(f), although not the "Possible fix" line. But (IMO) further unification should be another PR.

For this one, I guess we need to decide whether to do this at all. And whether the style here is what's wanted?

Does the module colouring make sense? The module which owns the function gets no colour (excepts its sub-modules are blue), while "foreign" modules extending it are given colours like the stack traces (but starting cool instead of warm).
Do bold types look OK on most people's screens?
"MethodError: ... ambiguous" uses an option to print the module & line number on the same line as the method. This is not used in stack traces, should it be used in methods(f)?
The length of the path & filename shown should be the same as for stack traces. This is quite long for e.g. LinearAlgebra: the error from det([1 2 3; 4 5 6]) wraps at 132 characters for me. Perhaps we should revisit that.

I fixed some doctests but there are some more... I'd rather not do that too many times.

simeonschaub · 2021-05-25T10:00:13Z

This already looks great, sorry it got forgotten a bit! This seems preferable to #40916 since it is more consistent with stacktrace printing.

* Does the module colouring make sense? The module which owns the function gets no colour (excepts its sub-modules are blue), while "foreign" modules extending it are given colours like the stack traces (but starting cool instead of warm).

Oh, that's smart! Sounds reasonable to me.

* Do bold types look OK on most people's screens?

Looks good for me, I wouldn't expect this to be a big issue.

* "MethodError: ... ambiguous" uses an option to print the module & line number on the same line as the method. This is not used in stack traces, should it be used in `methods(f)`?

Maybe? Although I don't think the more compact printing is bad here either.

* The length of the path & filename shown should be the same as for stack traces. This is quite long for e.g. LinearAlgebra: the error from `det([1 2 3; 4 5 6])` wraps at 132 characters for me. Perhaps we should revisit that.

Do you have a screenshot of how this looks like for you? This probably isn't a dealbreaker though.

I fixed some doctests but there are some more... I'd rather not do that too many times.

Do you know about #40546 (comment)? You can even pass revise=true to do this without having to rebuild the whole sysimage.

mcabbott · 2021-05-25T14:19:42Z

Do you know about #40546 (comment)?

Now I do, thanks! Will try that out.

Do you have a screenshot

Yes. This is the error printing with the pre-built Julia 1.6 -- some a long paths, and not useful ones. But perhaps orthogonal to this PR, which just re-uses the same path logic as for stack traces.

simeonschaub · 2021-05-26T13:24:38Z

doc/src/manual/methods.md

 ...
-[180] +(a, b, c, xs...) in Base at operators.jl:424
+ [207] +(a::Any, b::Any, c::Any, xs::Any...)


Was this change intentional? I think always printing the ::Any is a bit verbose and I am not sure it adds much information.

I agree it's a bit verbose in cases like that. What it was meant to do was to help cases like this, where otherwise untyped arguments go missing:

I think it would be fine to omit the ::Any here, but I don't have any strong opinions on this.

Here are some variants which print the module & path without a new line. (Because the new one above takes up a lot of lines compared to the old. And the types printed here are usually shorter than those in the stack traces.) I think this might be better.

On the left I omit the ::Any, what I don't like is that suddenly the variable name has to stand on its own, instead of being clearly secondary information to the signature. You can't answer "how many arguments?" by scanning "how many bold things?" anymore.

On the right, I've also added bold to required keywords, shown with cumsum. Seems like useful information to mark them somehow, hope bold isn't confusing --- all the types are also requirements, after all.

tecosaur · 2021-05-26T19:41:58Z

I made #40916 before someone pointed this out to me. I have no idea what's going to happen, but should this be the approach that ends up being used, I'd like to add some thoughts:

I'm not a fan of the two line version. If I'm looking at something with ~200 methods (like +) I'd rather have 200 lines to look at than 400. I think an "intelligent" switch could make sense though (if num methods < val, etc.)
I think argument names deserve more emphasis, in my PR I use the magenta colour for them. for example there's the join method with signature join(strings, delim, last) and I think it's worth applying emphasising argument names for cases such as those. If nothing else, since names are usually much shorter than type signatures this also makes it easier to count arguments, and resolves the ::Any problem.
I really like how modules are given per-module colouring
This may be outside the scope of this PR, but I think it's worth considering applying terminal hyperlinks to file names printed (in the terminal) by Julia

mcabbott · 2021-05-27T02:56:56Z

I think I now agree that one line is better, here. The types are typically much shorter than in stack traces. And moving the coloured module names to the right doesn't seem weird, especially since only a subset of them are coloured.
The signature which identifies the method is these types, while the variable names are just some details of the source code. It's good to show both but I do think the emphasis should be on the types. If you are looking at methods(f), I think it's often to see precisely what will match (or why it won't, or whether it will create ambiguity) rather than for general help.
That said, there's room for more exploration of syntax highlighting in general. I think OhMyREPL only touches things you type; but perhaps there should be hooks to make it easier to also change how errors (and the printing of Expr) etc. are printed?
Something like this hyperlinking would be great, here and in stack traces. Especially if it meant you could not print the whole path, which is adding a lot of clutter to all these examples. But I think this should be an independent PR.

mcabbott · 2021-05-27T03:42:40Z

base/methodshow.jl

+        for kw in kwargs
+            skw = sym_to_string(kw)
+            if QuoteNode(kw) in m.roots # then it's required
+                printstyled(io, skw, color=:bold)
+            else
+                print(io, skw)


This is meant to check which keyword arguments are required, and print those in bold.

Will this be robust? I don't know any details of the method struct, but in cases I could think of this appears to work.

Picture of this, BTW. Left is master, right this PR.

This question of whether the check for required keyword arguments is correct, safe, could use a look from someone who knows about things.

no, this is not robust. but quite amusing that you found this

Ok. I just poke around... Is there a robust way to query this?

I don't know of one right now. Let's leave that for later work?

tecosaur · 2021-05-27T05:13:29Z

Cool
I'm not sure my thinking lights up with yours. If we were in black & white you could only emphasise one thing, but by using a colour you can brightly label multiple, and the eye can more easily jump to what you're looking for. See my PR for an example.
So do I, personally think it would be great if something like what I mock up in Add colour to show(::Method) #40916 (comment) could happen, and OhMyREPL was included in Julia
I think this would make sense as an independent PR, yes. Affects this though, so I'll mention it here. If someone is willing to help me out (answer questions like which file should I put it in, how do I find X, ...) I'd like to do this as a learning experience for this codebase, since I like the idea of contributing to Julia 🙂 in particular I love how Julia has embraced Unicode, but I think it could benefit from going a bit further embracing colour and other features (like this)

mcabbott · 2021-05-27T14:26:13Z

Yes, it's true we aren't constrained to greyscale. I'm hesitant to touch the error printing, though... would it be weird if they diverge, is the information more or less useful in one than the other?

Here's what variables in :magenta looks like; this colour is not used for modules in methods(f) at the moment:

I quite like this, but perhaps it's too fancy? (This isn't committed yet.)

I think the code which is printing the path is in errorshow.jl, this moves it into a function print_module_path_file but leaves it there. The path gets altered by fixup_stdlib_path in methodshow.jl. It's all pretty messy, but with using Revise; Revise.track(Base) you can fairly easily mess with it and see what breaks.

mcabbott · 2022-01-19T13:12:06Z

Bump, pre-1.8. Pinging @StefanKarpinski maybe?

vtjnash · 2022-01-19T22:45:07Z

base/methodshow.jl

+        for kw in kwargs
+            skw = sym_to_string(kw)
+            if QuoteNode(kw) in m.roots # then it's required
+                printstyled(io, skw, color=:bold)
+            else
+                print(io, skw)


no, this is not robust. but quite amusing that you found this

vtjnash · 2022-01-19T22:45:17Z

base/methodshow.jl

-        file, line = updated_methodloc(m)
-        print(io, " at ", file, ":", line)
+
+    # module & flie, re-using function from errorshow.jl


Suggested change

# module & flie, re-using function from errorshow.jl

# module & file, re-using function from errorshow.jl

vtjnash · 2022-01-19T22:46:34Z

base/methodshow.jl

+    modulecolordict = Dict{Module, Symbol}()
+    modulecolorcycler = Iterators.Stateful(Iterators.cycle(METHODLIST_MODULECOLORS))
+    modulecolordict[parentmodule_before_main(modul)] = :blue


Is this really our only strategy for this? Global state through a particular list?

I have no memory of writing this :/ I presume I tried to copy the stack traces.

The list [:cyan, :green, :yellow] is global, but isn't modulecolorcycler local to this function?

ah, true, seems to be a copy from there. I guess this is okay

vtjnash · 2022-01-19T22:47:28Z

doc/src/devdocs/inference.md

@@ -96,7 +96,7 @@ Each statement gets analyzed for its total cost in a function called
 as follows:
 ```jldoctest; filter=r"tuple.jl:\d+"
 julia> Base.print_statement_costs(stdout, map, (typeof(sqrt), Tuple{Int},)) # map(sqrt, (2,))
-map(f, t::Tuple{Any}) in Base at tuple.jl:179
+map(f::Any, t::Tuple{Any}) @ Base tuple.jl:218


this seems like a regression

Can you explain what you mean?

I just landed here by chasing errors, and have not looked for the code which generates this. I believe that Base.Math.throw_complex_domainerror(:sqrt::Symbol, %2::Float64)::Union{} just below in this output does not take the path this PR changes, since it does not get printed in bold:

Is there an alternative path that the first line's printing should take? Or should I add some option like fancy=false (or checking IOContext) to the basic show(method) path, to print this closer to the old style?

The added ::Any is unnecessary, and seems to be explicitly being added in this PR

Ok, I can surely figure out a way to remove it here.

But the logic of including it in general is this:

https://user-images.githubusercontent.com/32575566/119697648-6b188f00-be1e-11eb-91e3-c7ae8612f2e4.png

On the right you can count the arguments by counting the bold words. On the left the grey f, is really easy to miss, and seems worse than before.

vtjnash

It looks like this seems sound. I have mentioned a few places to fix (to not change), but otherwise seems like it successfully shares code, so it quite sane.

JeffBezanson · 2022-01-20T20:44:51Z

From triage: after much discussion, we all came to a different conclusion, that we like having the signature and location on two lines since (1) it's nice to be consistent with stack traces, and (2) the method location is actually always quite important and two lines makes it much easier to scan the locations. We also like omitting ::Any; it's less noisy and matches how we expect people to write such method signatures in their code.

Sorry about all the back-and-forth on this; these formatting PRs can be really contentious as you know :)

mcabbott · 2022-01-20T20:53:40Z

Oh I know.

One question though is whether the all-important f fist argument goes missing too easily, light grey and no type. That's what the ::Any was trying to address. Another way might be to make all the arguments hot pink, a bit like #40251 (comment) . Strong feelings on this?

Fixing the code is easy, fixing the tests may take me a while to get to, we'll see.

vtjnash · 2022-01-20T21:08:53Z

Could we add some logic that prints either <light>::<bold> or <normal>, depending on whether there is content to print on the right-hand side?

mcabbott · 2022-01-20T21:31:35Z

Yes, surely. Will try that out.

tecosaur · 2022-01-21T03:01:09Z

Jeff/Jameson as a fan of making the arg name stand out regardless (as in #40251 (comment)), is there anything you have against that?

vtjnash · 2022-01-21T03:49:47Z

I mildly dislike the extra color, on the basis of being too noisy.

vtjnash · 2022-01-21T03:51:39Z

Note: closes #40913

tecosaur · 2022-01-21T03:53:37Z

I mildly dislike the extra color, on the basis of being too noisy.

I see. I'd comment that having it be styled the same regardless is good for visual consistency and it isn't unimportant enough to be greyed out, as the naming can often be informative.

JeffBezanson · 2022-01-21T04:51:01Z

Ah, I forgot to mention that @staticfloat made the point that we shouldn't rely too heavily on colors; the output should be nice even with the colors stripped. That's another reason I prefer leaving out ::Any.

JeffBezanson added the display and printing Aesthetics and correctness of printed representations of objects. label Mar 29, 2021

mcabbott force-pushed the methodshow branch from e727ecf to 8d85734 Compare May 20, 2021 01:55

thofma mentioned this pull request May 22, 2021

Colourising Method representation for easier glancing #40913

Closed

tecosaur mentioned this pull request May 22, 2021

Add colour to show(::Method) #40916

Closed

oscardssmith changed the title ~~Print methods(f) much like stack traces~~ Print methods(f) with color like stack traces May 22, 2021

oscardssmith added the REPL Julia's REPL (Read Eval Print Loop) label May 22, 2021

mcabbott marked this pull request as ready for review May 23, 2021 02:44

simeonschaub added the forget me not PRs that one wants to make sure aren't forgotten label May 26, 2021

simeonschaub added this to the 1.7 milestone May 26, 2021

simeonschaub reviewed May 26, 2021

View reviewed changes

mcabbott commented May 27, 2021

View reviewed changes

oscardssmith removed this from the 1.7 milestone May 27, 2021

mcabbott added 13 commits January 18, 2022 21:37

whitespace

b55aa9f

docstrings

e5e5f6e

fix some tests

228b695

more tests

51623eb

fix 41928

c3199c5

fix many tests, a few broken

7c369d3

revert some html cases

ccf6109

fix colon on methods(:)

dbc8204

typo

df8ce52

simplify path in manual

641dd79

three tests

b89e82c

change text/html to match

439789f

two tests in replcompletions

b563ee6

mcabbott force-pushed the methodshow branch from 1ccbcd4 to b563ee6 Compare January 19, 2022 02:55

vtjnash reviewed Jan 19, 2022

View reviewed changes

vtjnash approved these changes Jan 19, 2022

View reviewed changes

vtjnash mentioned this pull request Feb 22, 2022

Can we prettify how methods are printed as in #36026? #36129

Closed

Seelengrab mentioned this pull request Apr 24, 2022

Add stacktrace colors to showing MethodErrors and method lists #45069

Merged

oscardssmith closed this in #45069 Jun 24, 2022

oscardssmith removed the forget me not PRs that one wants to make sure aren't forgotten label Sep 15, 2022

	# module & flie, re-using function from errorshow.jl
	# module & file, re-using function from errorshow.jl

Print methods(f) with color like stack traces #40251

Print methods(f) with color like stack traces #40251

Conversation

mcabbott commented Mar 29, 2021 • edited by vtjnash Loading

thchr commented Mar 29, 2021 • edited Loading

mcabbott commented Mar 29, 2021 • edited Loading

fingolfin commented Mar 30, 2021

mcabbott commented Mar 30, 2021

fingolfin commented Mar 30, 2021

mcabbott commented Mar 30, 2021 • edited Loading

StefanKarpinski commented Mar 31, 2021

fingolfin commented Mar 31, 2021

thchr commented Mar 31, 2021

mcabbott commented Mar 31, 2021

JeffBezanson commented May 24, 2021

mcabbott commented May 24, 2021 • edited Loading

simeonschaub commented May 25, 2021

mcabbott commented May 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tecosaur commented May 26, 2021

mcabbott commented May 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tecosaur commented May 27, 2021

mcabbott commented May 27, 2021 • edited Loading

mcabbott commented Jan 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcabbott Jan 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vtjnash left a comment

Choose a reason for hiding this comment

JeffBezanson commented Jan 20, 2022

mcabbott commented Jan 20, 2022

vtjnash commented Jan 20, 2022

mcabbott commented Jan 20, 2022

tecosaur commented Jan 21, 2022

vtjnash commented Jan 21, 2022

vtjnash commented Jan 21, 2022

tecosaur commented Jan 21, 2022

JeffBezanson commented Jan 21, 2022

Print `methods(f)` with color like stack traces #40251

Print `methods(f)` with color like stack traces #40251

mcabbott commented Mar 29, 2021 •

edited by vtjnash

Loading

thchr commented Mar 29, 2021 •

edited

Loading

mcabbott commented Mar 29, 2021 •

edited

Loading

mcabbott commented Mar 30, 2021 •

edited

Loading

mcabbott commented May 24, 2021 •

edited

Loading

mcabbott commented May 27, 2021 •

edited

Loading

mcabbott commented May 27, 2021 •

edited

Loading

mcabbott Jan 19, 2022 •

edited

Loading