Closed myronmarston closed 8 years ago
@myronmarston You actually did uncover a bug in how I was handling the ISO triplet stuff, so I'm glad you reported the issue you found!
The iso_triplet
is a standard form for {YYYY, WW, WD}
, so I think I'd rather treat that as a documentation issue. However, the current docs do describe the tuple returned, do you have suggestions on how the docs could be improved?
You can use {{^y, ^m, ^d}, _} = {y, m, d} |> Date.from |> DateConvert.to_erlang_datetime
to effectively roundtrip the conversion. However you will get the time component as well (hence the match on the result of the conversion to get the date back). Right now I don't have a specific function for taking in a {YYYY, MM, DD}
tuple and converting back to one, but you can get it trivially via {date, _} = DateConvert.to_erlang_datetime(%DateTime{})
However, the current docs do describe the tuple returned, do you have suggestions on how the docs could be improved?
The docs explain this. However, as someone who thought he already knew what iso_triplet
meant, I didn't initially read them, and then got really confused with what I was seeing...and then I read the docs, and realized there was a part of iso8601 I had never dealt with and didn't know existed :).
Maybe the { YYYY, WW, WD }
form is common in the community, but I had never seen or heard of that form before. My experience with other date APIs (e.g. Ruby's Date.iso8601
) made me think of iso8601 exclusively in terms of YYYY-MM-DD form (which led me to think the triplet was { YYYY, MM, DD }
). The wikipedia article on iso8601 uses "date" to refer to the "YYYY-MM-DD" format I expect and "Date with week number" or "week date" to refer to what you are calling iso_triplet
-- which suggests that putting "week" in the function names somewhere would more closely align with common terminology usage.
All that said: changing the name of a public API is a hard sell as it's disruptive to the community so I can completely understand wanting to keep them as is. Or maybe I'm the odd one here (as a newcomer to elixir) and most elixir users would understand iso_triplet
as you treat it.
Regardless, thanks for the great library :). Timex is working well for me besides the couple of issues I reported.
I can definitely see the potential for confusion, and I'd be tempted to rename it. However, I think the reason why I feel it's less ambiguous than stated is because the usual meaning of ISO vs not-ISO (when it comes to conversions), has to do with the fact that the ISO calendar represents dates in terms of {year, week, weekday}
, not {year, month, day}
. This is because ISO weeks/days do not always occur in the same year you'd expect. Using the example from #71, {2014, 1, 1}
is actually the date 2013-12-30
, because the first day of 2014 according to the ISO calendar is actually in December of 2013. So having a function called iso_triplet
which takes a DateTime and produces a {year, month, day}
tuple shouldn't be called iso_triplet
to begin with as it is effectively redundant (and doesn't tell you much about what is returned, at least without checking the docs), it would make more sense to call it to_date_tuple
or to_erl
(since the triplet of that form is one of the Erlang datetime types).
As far as alternatives go to make it less confusing, my preference would be more something along the lines of Date.to_iso_calendar
, since the dates on the ISO calendar take the form of {year, week, weekday}
.
Thoughts?
However, I think the reason why I feel it's less ambiguous than stated is because the usual meaning of ISO vs not-ISO (when it comes to conversions), has to do with the fact that the ISO calendar represents dates in terms of
{year, week, weekday}
, not{year, month, day}
.
I did not know about how the ISO calendar represents dates! I'm probably not the best person to ask about this because my knowledge of ISO is very limited (I've always just thought of it as the name of the YYYY-MM-DD
format for dates). I've never read the standard.
Maybe you can get feedback from other people who are more familiar with the ISO standard?
@myronmarston Well it's the feedback from users who aren't as familiar that is more important I think :), the reason being that someone who was familiar designed the API, so if it's confusing, then I'd rather get input from those who have less context clouding their opinion. Do you think documenting some of these things would make sense? Would people read documentation on, say, what the ISO calendar is and how Timex APIs relate to it?
Maybe a top-level note in the README (and in the Timex.Date
module doc) saying something like this would suffice?
Warning: Timex functions with
iso
in the name refer to how the ISO calendar represents dates and not theYYYY-MM-DD
ISO8601 date format.
Included in Version 2.
I found the
iso_triplet
functions related functions and (wrongly) assumed they were referring to{year, month, day}
tuples -- after all, an ISO-8601 date is in the formYYYY-MM-DD
. I see now that the ISO 8601 standard also supports a week date:https://en.wikipedia.org/wiki/ISO_week_date
...but I had never heard about that before, and was quite confused about the
iso_triplet
functions when I first tried to use them. I suspect it's a common mistake others will make. What do you think about renaming theiso_triplet
functions to something likeiso_week_date_triplet
or similar to make it clear that it's not the{ year, month, date}
triplets someone might expect?Also, I see that I can construct a timex date time from a
{ year, month, date }
triplet usingTimex.Date.from
but I don't see a way to convert back to that form with any of the provided functions. Am I just not seeing it or is that not part of the API provided by Timex? Would you consider adding it?Thanks!