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

RFC: More printing overhaul #169

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Keno
Copy link

@Keno Keno commented Jul 10, 2021

This is sort of an expansion of #168. There I tried to keep the PR
fairly minimal to only fix the specific case that was bothering me.
Here I try a more extensive set of changes to show some options of
what might be possible. Some subset of these may be interesting, so
I'm opening this for discussion with the maximum set of possible
changes and we can discuss from there.

First things first, here's some demo printing:

julia> a = Interval{Closed, Open}(DateTime(2013, 2, 13), DateTime(2013, 2, 13, 1))
[2013-02-13T00:00:00, 2013-02-13T01:00:00)

julia> show(a)
Interval{Closed,Open}(DateTime("2013-02-13T00:00:00"), DateTime("2013-02-13T00:00:00"))

julia> show(IOContext(stdout, :compact=>true), a)
i`[DateTime("2013-02-13T00:00:00"),DateTime("2013-02-13T01:00:00"))`

julia> i`[DateTime("2013-02-13T00:00:00"),DateTime("2013-02-13T01:00:00"))`
[2013-02-13T00:00:00, 2013-02-13T01:00:00)

julia> const dB = Unitful.dB
dB

julia> 1 dB .. 2 dB
1 dB .. 2 dB

julia> i`(1dB, 2dB]`
(1 dB, 2 dB]

The most obvious change, is probably the introduction of the i_cmd
macro that parses back the short interval syntax and is used for
compact, parseable print. Here's the feature highlights of this scheme:

  • Compact printing is changed to use the unicode double-wide brackets,
    to distinguish it from ascii brackets, which already have a different meaning

  • show(::IO, x::Interval) is always parseable, both in compact mode
    and in non-compact mode. Note that the compact interval printing
    changed from using .. to ,. This is to clarify the parsing rules
    inside the macro. In particular, other than the first and last character,
    the macro uses tuple parsing rules, so you can do things like:

julia> i`(now(), now()+Hour(1)]`
(2021-07-10T02:51:37.780, 2021-07-10T03:51:37.780]
  • The i-macro parses both ASCII and double-wide brackets as interval bounds

  • The parseable and non-parseable printing now generally only differs in
    the printing or non-printing of the i-macro. This is quite useful to
    get something parseable back from an interval that was printed in
    non-parseable context.

  • Closed intervals have special printing using the .. operator to make the
    output match the input syntax. Of course they can also be constructed using
    the i-macro (but will still be printed as ..).

  • Even for non-compact, parseable prinitng, the long type parameter is dropped,
    in favor of only showing the Open/Closed setting, since the type should be
    redunant with the parsing of the bounds being printed.

As I said, several of these are independent, so we might only want a subset
of these, but I thought, I'd present it as one big thing.

This doesn't have any doc(test) changes yet, since I figure things will
change, and I didn't want to bother doing that twice :).

This is sort of an expansion of invenia#168. There I tried to keep the PR
fairly minimal to only fix the specific case that was bothering me.
Here I try a more extensive set of changes to show some options of
what might be possible. Some subset of these may be interesting, so
I'm opening this for discussion with the maximum set of possible
changes and we can discuss from there.

First things first, here's some demo printing:
```
julia> a = Interval{Closed, Open}(DateTime(2013, 2, 13), DateTime(2013, 2, 13, 1))
[2013-02-13T00:00:00, 2013-02-13T01:00:00)

julia> show(a)
Interval{Closed,Open}(DateTime("2013-02-13T00:00:00"), DateTime("2013-02-13T00:00:00"))

julia> show(IOContext(stdout, :compact=>true), a)
i`[DateTime("2013-02-13T00:00:00"),DateTime("2013-02-13T01:00:00"))`

julia> i`[DateTime("2013-02-13T00:00:00"),DateTime("2013-02-13T01:00:00"))`
[2013-02-13T00:00:00, 2013-02-13T01:00:00)

julia> const dB = Unitful.dB
dB

julia> 1 dB .. 2 dB
1 dB .. 2 dB

julia> i`(1dB, 2dB]`
(1 dB, 2 dB]
```

The most obvious change, is probably the introduction of the i_cmd
macro that parses back the short interval syntax and is used for
compact, parseable print. Here's the feature highlights of this scheme:

- Compact printing is changed to use the unicode double-wide brackets,
  to distinguish it from ascii brackets, which already have a different meaning

- `show(::IO, x::Interval)` is always parseable, both in compact mode
  and in non-compact mode. Note that the compact interval printing
  changed from using `..` to `,`. This is to clarify the parsing rules
  inside the macro. In particular, other than the first and last character,
  the macro uses tuple parsing rules, so you can do things like:

```
julia> i`(now(), now()+Hour(1)]`
(2021-07-10T02:51:37.780, 2021-07-10T03:51:37.780]
```

- The i-macro parses both ASCII and double-wide brackets as interval bounds

- The parseable and non-parseable printing now generally only differs in
  the printing or non-printing of the i-macro. This is quite useful to
  get something parseable back from an interval that was printed in
  non-parseable context.

- Closed intervals have special printing using the `..` operator to make the
  output match the input syntax. Of course they can also be constructed using
  the i-macro (but will still be printed as `..`).

- Even for non-compact, parseable prinitng, the long type parameter is dropped,
  in favor of only showing the Open/Closed setting, since the type should be
  redunant with the parsing of the bounds being printed.

As I said, several of these are independent, so we might only want a subset
of these, but I thought, I'd present it as one big thing.

This doesn't have any doc(test) changes yet, since I figure things will
change, and I didn't want to bother doing that twice :).
@Keno Keno requested a review from omus as a code owner July 10, 2021 03:02
Copy link
Collaborator

@omus omus left a comment

Choose a reason for hiding this comment

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

Compact printing is changed to use the unicode double-wide brackets,
to distinguish it from ascii brackets, which already have a different meaning

I can definitely get behind this.

@@ -258,35 +258,103 @@ function Base.convert(::Type{T}, interval::Interval{T}) where T
end
end

##### Interval Macro #####

macro interval_cmd(str)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason you used *_cmd over *_str?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably be using parse(Interval, ...) under the hood and unify the logic

Would probably need to expand the existing functionality with:

function Base.parse(::Type{Interval}, str::AbstractString; element_parser=(T, el) -> eval(Meta.parse(el))::T)
    x = invoke(parse, Tuple{Type{Interval{Any}}, AbstractString}, Interval{Any}, str; element_parser)
    T = promote_type(typeof(x.first), typeof(x.last))
    return convert(Interval{T}, x)
end

function Base.convert(::Type{Interval{T}}, interval::Interval) where T
    L, R = bounds_types(interval)
    return Interval{T,L,R}(first(interval), last(interval))
end

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.

2 participants