Skip to content

Simplify #inspect #12

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

Jump to bottom
Merged
jeremyevans merged 2 commits into from
Oct 18, 2019
Merged

Simplify #inspect #12

jeremyevans merged 2 commits into from
Oct 18, 2019

Conversation

@zverok
Copy link
Contributor

@zverok zverok commented Aug 11, 2019

I'd like to raise a discussion on the following change:

# before:
Date.new(2010, 1, 3).inspect
# => #<Date: 2010-01-03 ((2455200j,0s,0n),+0s,2299161j)> 

# after:
Date.new(2010, 1, 3).inspect
# => #<Date: 2010-01-03>

Justification: for most of contexts where Date could be used (web apps, data conversion scripts, experiments and so on), the "((2455200j,0s,0n),+0s,2299161j)" suffix is meaningless, and just obscures the data reading. Imagine the (pretty simple) example:

# before
(1..10).map { |days| Date.today - days }
# => [#<Date: 2019-08-10 ((2458706j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-09 ((2458705j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-08 ((2458704j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-07 ((2458703j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-06 ((2458702j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-05 ((2458701j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-04 ((2458700j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-03 ((2458699j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-02 ((2458698j,0s,0n),+0s,2299161j)>, #<Date: 2019-08-01 ((2458697j,0s,0n),+0s,2299161j)>]

# after:
(1..10).map { |days| Date.today - days }
 => [#<Date: 2019-08-10>, #<Date: 2019-08-09>, #<Date: 2019-08-08>, #<Date: 2019-08-07>, #<Date: 2019-08-06>, #<Date: 2019-08-05>, #<Date: 2019-08-04>, #<Date: 2019-08-03>, #<Date: 2019-08-02>, #<Date: 2019-08-01>]

Whether it is a list of dates we extract from DB, or have in test (of some scheduling library), or parse from CSV, I believe the "before" example is very hard to read and provides no additional value.

On the other hand, for the rare contexts when values like "julian day number" matter, the ((2458697j,0s,0n),+0s,2299161j) format seem to be too terse to be easily used by anybody not 200% familiar with library's internals.

@jeremyevans
Copy link
Contributor

jeremyevans commented Oct 2, 2019

I agree this makes more sense and will make the inspect output easier to read. I plan to merge this before 2.7 is released unless there are objections.

@jeremyevans jeremyevans merged commit 8c02586 into ruby:master Oct 18, 2019
@zverok zverok deleted the simplify-inspect branch October 18, 2019 20:18
@nurse
Copy link
Member

nurse commented Oct 25, 2019

I object this change.
inspect should suggest its internal and it should keep julian day number.

If you want to introduce such Web friendly date library, don't build it on top of current date library.
Please create new one on top of Time.

@jeremyevans
Copy link
Contributor

jeremyevans commented Oct 25, 2019

@nurse I can revert this, however, first please consider that "inspect should suggest its internal" is no longer a reason to do so. The date library since 1.9.3 does not store the julian day number for dates in the typical case where it is initialized via Date.new/Date.civil or Date.today. If, after considering that, you would still like this reverted, please let me know and I will revert this.

@nurse
Copy link
Member

nurse commented Oct 26, 2019

@jeremyevans The fact it doesn't calculate julian day number is implementation detail.
Of course you have a question, what is the difference between "internal" and "implementation detail".

I think Date#inspect should show the difference between Date.today(Date::ITALY) and Date.today(Date::ENGLAND).

# master
irb(main):002:0> Date.today(Date::ITALY)
=> #<Date: 2019-10-27>
irb(main):003:0> Date.today(Date::ENGLAND)
=> #<Date: 2019-10-27>

# 2.6
irb(main):002:0> Date.today(Date::ITALY)
=> #<Date: 2019-10-27 ((2458784j,0s,0n),+0s,2299161j)>
irb(main):003:0> Date.today(Date::ENGLAND)
=> #<Date: 2019-10-27 ((2458784j,0s,0n),+0s,2361222j)>

@jeremyevans
Copy link
Contributor

jeremyevans commented Oct 26, 2019

@nurse OK. I'll revert this change.

@jeremyevans
Copy link
Contributor

jeremyevans commented Oct 27, 2019

Revert committed at 875d563.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zverok @jeremyevans @nurse