kipcole9/calendrical

Calendars and Calendar Calculations for Elixir

calendrical's Issues

Wrong codepoints for superscripts

Module Calendrical.Math.Fraction in file math.ex.
From the @doc of function to_string:

  ## Example
      iex> Calendrical.Math.Fraction.to_string {2, {16, 25}}
      "2ⁱ⁶⁄₂₅"

The numerator contains the character "i" instead of "1".
The codepoint for digit "3" is missing.
These changes of function superscript help:

@spec superscript(integer) :: String.t
- def superscript(digit) when digit in [0,1,4,5,6,7,8,9] do   # always catches digit '1'
+ def superscript(digit) when digit in [0,4,5,6,7,8,9] do     # `1` deleted
    <<226, 129, 176 + digit>>
  end
- def superscript(1), do: "\u2071"    # this produces character "ⁱ" instead of digit "¹"
+ def superscript(1), do: "\u00b9"    # correct codepoint
  def superscript(2), do: "\u00b2"
+ def superscript(3), do: "\u00b3"    # additional codepoint

Some general feedback

Here are some notes, after having glanced over the library during breakfast.

• Calendrical:

  • The naming of days/1 for a function that returns the number of what day 'monday' is feels counter-intuitive to me. Maybe weekday_number/1 or something similar is better. Besides, the names of the weekdays (and even if there are seven days inside) depend on the actual calendar and language used, so maybe this should better be moved to another module (Such as Kday).
  • Probably, the datetime-geared functions should be removed. %DateTime{} stores the timezone offset on top of what NaiveDateTime does. At least Elixir's standard library takes the approach of having developers explicitly think about this timezone management (and thus always having the explicit NaiveDateTime -> DateTime conversion step).
    • For this library it is unfortunate that some of the RataDie-calculations inside Elixir's standard library are managed in private functions. Maybe an alternate idea would be to create a %RataDie{} calendar implementation (with trivial naive_datetime_to_rata_die and naive_datetime_from_rata_die-implementations).

• Calendrical.Math:

  • At least the angles_to_radians functionality is not required by any calendars, as far as I know (or is it? 😁 )
  • It is worth noting that Integer (courtesy of the Calendar additions with the Rata Die day fraction calculations) now contains a gcd/2.

• Calendrical.JulianDay:

  • line 58 contains a magic number. Probably better to create a constant for the middle of the day.
  • I don't actually think there is a reason to work with microsecond-precision day fractions here. You could just use the lowest fraction that allows you to express something. For noon, this would be {1, 2}.
  • Line 84 might convert the result to a float, which is probably not what you want.
  • Why is Calendrical.JulianDay not a Calendar behaviour implementation?

• Calendrical.Kday:

  • Seems fine, although it is been a while since I read the K-day part of the Calendrical Calculations paper. 😅

Thank you for writing this! ❤️

Compilation error in latest calendrical version

I downloaded and unzipped 'calendrical' into a new directory. Then I did a 'mix deps.get' which was OK. But when I did a 'mix do clean, compile' I got a compilation error:

`D:\Projekte\Chronos\calendrical>mix do clean, compile
Compiling 12 files (.ex)
warning: function naive_datetime_from_iso_days/1 required by behaviour Calendar is not implemented (in module Calendrical.Calendar.Armenian)
  lib/calendrical/calendar/armenian.ex:1


== Compilation error in file lib/calendrical/calendar/julian.ex ==
** (UndefinedFunctionError) function Calendar.ISO.naive_datetime_to_rata_die/7 is undefined or private. Did you mean one of:

      * naive_datetime_from_iso_days/1
      * naive_datetime_to_iso8601/7
      * naive_datetime_to_iso8601/8
      * naive_datetime_to_iso_days/7
      * naive_datetime_to_string/7

    (elixir) Calendar.ISO.naive_datetime_to_rata_die(0, 12, 30, 0, 0, 0, {0, 6})
    lib/calendrical/calendar/julian.ex:6: (module)
    (stdlib) erl_eval.erl:670: :erl_eval.do_apply/6`

Alternate calculation of Julian leap year

May I suggest to replace the functions

  def leap_year?(year) when year > 0 do
    Math.mod(year, 4) == 0
  end

  def leap_year?(year) do
    Math.mod(year, 4) == 3
  end

in module Calendrical.Calendar.Julian by an alternative that doesn't use the Math helper module and uses only one function:

def leap_year_alt?(year) do
    (year > 0) && (rem(year, 4) == 0) || (rem(year, 4) == -1)
  end

Afterall we know that year is an integer. The term (rem(year, 4) == -1) takes into account that Erlang produces a negative modulo for negative numbers. In other languages (i.e. Python) this would have been (year % 4) == 3 for negative years.

I tested the new function against the old one using:

  test "Julian.leap_year?(year)" do
    for year <- -4717..2020 do
      assert Julian.leap_year_alt?(year) === Julian.leap_year?(year)
    end
  end

All tests produced the same results and passed.
Of course, if accepted, leap_year_alt?, should be renamed leap_year?, so that no outer depencies will be broken.