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

Edit of "Output-function summary" docs from a new learner #56767

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

nathanrboyer
Copy link
Contributor

@nathanrboyer nathanrboyer commented Dec 6, 2024

This is a refactor of the helpful output summary written recently by @stevengj in #54547. I think it makes the section more digestible to move some of the information that was contained within the bullets into separate parts, so the bullet points themselves are smaller. It may need to be edited for accuracy and formality.

@nsajko nsajko added the docs This change adds or pertains to documentation label Dec 6, 2024
Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

Thanks for offering this edit, @nathanrboyer!

Personally I have no preference for the proposed version over the previous version or vice versa. I'd be happy to help do a final review and merge if other folks think this is an improvement, though.

Comment on lines 1645 to 1646
Note that [`convert(String, x)`](@ref) is purposely not defined
so the user can be explicit about the string representation they desire.
Copy link
Member

Choose a reason for hiding this comment

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

I think that the real reason is that convert in Julia is used for implicit conversions, e.g. when assigning an expression to an container of a different type. So it is never defined for converting between conceptually different types of objects, e.g. between numbers and strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have now rewritten this sentence according to your explanation.

doc/src/manual/types.md Outdated Show resolved Hide resolved
doc/src/manual/types.md Outdated Show resolved Hide resolved
@stevengj stevengj added the io Involving the I/O subsystem: libuv, read, write, etc. label Dec 31, 2024
* [`show(io, mime, x)`](@ref), with three arguments,
performs verbose pretty-printing of `x`.
Multiple 3-argument `show` methods can be defined for various MIME types to enable
richer display of `x` in some interactive environments as discussed above.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
richer display of `x` in some interactive environments as discussed above.
richer display of `x` in some interactive environments as discussed below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, when I read this passage I thought "above" referred to the Polar example in the previous section. Now I think you may have been referring to the small explanation in the display bullet. I will rethink this.

Multiple 3-argument `show` methods can be defined for various MIME types to enable
richer display of `x` in some interactive environments as discussed above.
By default (if no 3-argument method is defined for `typeof(x)`),
it calls the 2-argument `show(io, x)`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it calls the 2-argument `show(io, x)`.
`show(io, "text/plain", x)` calls the 2-argument `show(io, x)`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sentence is copied from the original text, but I am thinking now it can just be deleted?

If not deleted, then maybe moved into the display bullet like this?

In the REPL, display calls show(io, MIME"text/plain", x) (which defaults to show(io, x) if not separately defined).

Copy link
Member

@stevengj stevengj Jan 2, 2025

Choose a reason for hiding this comment

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

It’s copied from the previous text, but you edited the preceding sentence so that the new qualification is needed.

I don’t think it can be deleted. It’s important to explain how 3-arg show defaults to 2-arg show (but only for text/plain)

is the default simple text representation of `x`.
It is typically the format you would employ to input `x` into Julia.

* [`show(io, mime, x)`](@ref), with three arguments,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* [`show(io, mime, x)`](@ref), with three arguments,
* [`show(io, ::MIME"text/plain", x)`](@ref), with three arguments,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not understand why you want this bullet to be specific to a text representation rather than explaining the 3-argument show generally.

Copy link
Member

Choose a reason for hiding this comment

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

Because text/plain is the common case, and most of this section is about text output . Most types don’t offer output for any other mime types, which is why they are only mentioned in passing here.

Also, if you want to overload it you need to know the specific signature.


* [`show(io, mime, x)`](@ref), with three arguments,
performs verbose pretty-printing of `x`.
Multiple 3-argument `show` methods can be defined for various MIME types to enable
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Multiple 3-argument `show` methods can be defined for various MIME types to enable
Additional 3-argument `show` methods can be defined for other MIME types to enable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is contingent on the decision above: mime vs ::MIME"text/plain".

performs verbose pretty-printing of `x`.
Multiple 3-argument `show` methods can be defined for various MIME types to enable
richer display of `x` in some interactive environments as discussed above.
By default (if no 3-argument method is defined for `typeof(x)`),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
By default (if no 3-argument method is defined for `typeof(x)`),
Although the underlying method is defined for a [`MIME`](@ref) argument, the caller can
simply pass a string such as `"text/plain"`, at the expense of a dynamic dispatch.
By default (if no 3-argument method is defined for `typeof(x)`),

Copy link
Contributor Author

@nathanrboyer nathanrboyer Jan 2, 2025

Choose a reason for hiding this comment

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

I think this is good to explain, but it leaves me with more questions than answers. Why is the string version used most places if it is slower?
See also this related issue I created: #56768

Copy link
Member

Choose a reason for hiding this comment

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

Internally, we should call the MIME version. For users, the string version is easier, and is rarely performance-critical.

@stevengj
Copy link
Member

stevengj commented Jan 1, 2025

See also #55860 for another tweak.

nathanrboyer and others added 4 commits January 2, 2025 11:32
Co-authored-by: Steven G. Johnson <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>
@nathanrboyer
Copy link
Contributor Author

See also #55860 for another tweak.

I added a form of that PR as below.

1612 (println and printstyled both call print,
1613 but add a newline or styling, respectively.)

doc/src/manual/types.md Outdated Show resolved Hide resolved
@nathanrboyer
Copy link
Contributor Author

@stevengj I took another pass at the summary. The bullets are longer now but hopefully more informative. Please review at your convenience.

My intent was to start by explaining each function/method generally and then suggest to the reader how/why they might wish to create new method definitions.

New types will automatically define `show(io, x)`
as the format employed to input `x` into Julia.
However, it may be desirable to overwrite this default
to define nicer printing for collections of the new type.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not totally confident on why when/why to overwrite 2-arg show. Printing inside a vector is the example used above for Polar, but the show docstring maintains that the definition "should be parseable Julia code when possible" which seems to already be the default definition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants