Skip to content

Conversation

wannaphong
Copy link
Member

@wannaphong wannaphong commented Nov 15, 2024

Thank @cstorm125 for the function name.

What does this changes

Add Thai Solar Date convert to Thai Lunar Date

Fixed #993

>>> from pythainlp.util import to_lunar_date
>>> from datetime import date
>>> 
>>> to_lunar_date(date(2025,2,25))
'แรม 13 ค่ำ เดือน 3'

Your checklist for this pull request

  • Passed code styles and structures
  • Passed code linting checks and unit test

Thank @cstorm125 for the function name.
@pep8speaks
Copy link

pep8speaks commented Nov 15, 2024

Hello @wannaphong! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-11-15 16:26:27 UTC

@coveralls
Copy link

coveralls commented Nov 15, 2024

Coverage Status

coverage: 52.515% (+0.4%) from 52.102%
when pulling 99df362 on add-thai-lunar-date
into 630b355 on dev.

@bact bact added the enhancement enhance functionalities label Nov 15, 2024
@bact
Copy link
Member

bact commented Nov 15, 2024

Few issues

  • There are some unused variables, see https://app.codacy.com/gh/PyThaiNLP/pythainlp/pull-requests/998/issues
  • Variable and function names should follow conventions in PEP 8 https://peps.python.org/pep-0008/#function-and-variable-names
    • For example, instead of StartY, use start_y.
  • Some variable names have a prefix i, probably as a convention from VBA to indicate that this variable is an Integer. Python doesn't have that convention. Actually, i here is for "input", still redundant (except in the context of i_date, since date is already used by Python)
    • So instead of iYear, we can use just year.
  • The use of unnecessary XLMod() instead of a standard % for modulo

Copy link
Member

@bact bact left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
20.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@wannaphong wannaphong merged commit c5de73c into dev Nov 15, 2024
36 of 40 checks passed
@wannaphong wannaphong deleted the add-thai-lunar-date branch November 15, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement enhance functionalities
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add Thai Solar Date convert to Thai Lunar Date
4 participants