Conversation
|
Thanks for the contribution. I think this would make an excellent addition to the library. I especially appreciate the details docstring. I see that there's a class ( Can you also add a |
|
Hi, dear Neil,
There are several versions of Tibetan calendar, with similar calculation methods but different epoch data. Currently I only implemented most official Phugpa version used widely by Tibetans inside and outside China esp. Gelugpa school. Another popular one is Tsurphu version. If I have time in the future, I might add an input argument or setting to choose versions.
I'd like to create a tibetan.rst. I am not familiar with RST format. It seems current files in docs/modules are very simple, for example:
.. automodule:: convertdate.armenian
:members:
:undoc-members:
Is this good enough?
Thanks,
David
Sent with [ProtonMail](https://protonmail.com/) Secure Email.
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
…On Saturday, January 29th, 2022 at 11:44 PM, Neil Freeman ***@***.***> wrote:
Thanks for the contribution. I think this would make an excellent addition to the library. I especially appreciate the details docstring.
I see that there's a class (Cal_school) that's being used once to compute the constants used in calculation. Is there a need for this to be a class? Could it be a function that returns the constants, or could we just make the constants global variables, which is a pattern used in other modules?
Can you also add a tibetan.rst file to docs/modules, following the pattern of the other libraries?
—
Reply to this email directly, [view it on GitHub](#53 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ALVETCGNFSLPBK6JCLRVKILUYQDMZANCNFSM5NCMUUEQ).
Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub).
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
I have added docs/tibetan.rst, please merge the latest one. |
|
any question? |
move import unittest before import convertdate per pylint.
|
fixed one pylint warning. |
| def to_jd(year, month, leap_month, day, leap_day): | ||
| '''Obtain Julian day for Tibetan date''' |
There was a problem hiding this comment.
Are the leap_month and leap_day arguments necessary here? Shouldn't the converter be able to check if the given month/day is leap?
|
I've pushed a commit (in a new branch) that refactors this slightly: 0a16a9b My main goal was to set the library up for future expansion without other changes. I added a method option to many function, which follows the pattern in other converters. So in effect one is calling this: To support this, I reduced the class to a dictionary (with a few other constants). Having all the constants available seems cleaner to me than using a class. Let me know what you think. |
|
Hi, Neil,
When converting a Tibetan date to gregorian (as well as to_jd), one has to specify a unique day with leap_month/leap_day information. For example, to_jd(2022, 1, leap_month=false, 11, leap_date=true) returns gregorian of 2022-03-13 while to_jd(2022, 1, leap_month=false, 11, leap_data=false) returns gregorian of 2022-03-14.
On the other direction, you can use from_gregorian(year, month, day) to check whether a day is leap_day/leap_month in Tibetan calendar.
Thanks,
David
Sent with [ProtonMail](https://protonmail.com/) secure email.
…------- Original Message -------
On Sunday, March 27th, 2022 at 5:44 AM, Neil Freeman ***@***.***> wrote:
@fitnr commented on this pull request.
---------------------------------------------------------------
In [src/convertdate/tibetan.py](#53 (comment)):
> +def to_jd(year, month, leap_month, day, leap_day):
+ '''Obtain Julian day for Tibetan date'''
Are the leap_month and leap_day arguments necessary here? Shouldn't the converter be able to check if the given month/day is leap?
—
Reply to this email directly, [view it on GitHub](#53 (review)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ALVETCFPGZHUGB5A4C3RN5TVB6ALNANCNFSM5NCMUUEQ).
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
Hi, Neil,
It is good of the refactor to have a consistent style for the whole package. I am fine with it.
Thanks,
David
Sent with [ProtonMail](https://protonmail.com/) secure email.
…------- Original Message -------
On Sunday, March 27th, 2022 at 7:32 AM, Neil Freeman ***@***.***> wrote:
I've pushed a commit (in a new branch) that refactors this slightly: [0a16a9b](0a16a9b)
My main goal was to set the library up for future expansion without other changes. I added a method option to many function, which follows the pattern in other converters. So in effect one is calling this: tibetan.from_gregorian(2022, 3, 26, method=tibetan.PHUGPA)
To support this, I reduced the class to a dictionary (with a few other constants). Having all the constants available seems cleaner to me than using a class.
Let me know what you think.
—
Reply to this email directly, [view it on GitHub](#53 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ALVETCHMZQREPGSBAIV7RR3VB6M7XANCNFSM5NCMUUEQ).
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
I have implemented a Phugpa version of Tibetan calendar, tested with many different days, and want to contribute to convertdate package instead of a standalone one.