-
-
Notifications
You must be signed in to change notification settings - Fork 561
Update _ThaiLunisolar and ThaiCalendarHolidays documentation
#2439
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.
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
Conversation
|
Caution Review failedThe pull request is closed. Summary by CodeRabbit
WalkthroughThe changes update the documentation for the Thai calendar classes. In the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
holidays/calendars/thai.py:594
- The return description in the docstring for boun_haw_khao_salark_date is incorrect. It should reference "Boun Haw Khao Salark" rather than "Pchum Ben".
Estimated Gregorian date of Pchum Ben.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2439 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 210 210
Lines 13348 13348
Branches 1909 1909
=========================================
Hits 13348 13348 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
holidays/calendars/thai.py:24
- [nitpick] Consider revising 'Work from 1913' to 'Works from 1913' for improved grammatical correctness.
'''Thai Lunar Calendar Holidays. ** Work from 1913 (B.E. 2456/2455) onwards until 2157 (B.E. 2700) as we only have
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
holidays/calendars/thai.py:310
- There appears to be a typo in 'Makha Buxha'; consider correcting it to 'Makha Bucha' for consistency with common naming conventions.
Also known as "Magha Puja", "Makha Buxha" and "Meak Bochea".
FYI that's the Laotian variant of the holiday name, I'm keeping it as-is for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Signed-off-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com>
|
What the heck copilot is doing here and where is the coderabbit? 🤷♂️ upd: okay, I see the CR but it seems we have been whitelisted for co-pilot recently. Or something else has changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
holidays/calendars/thai.py (1)
685-706: 🧹 Nitpick (assertive)Method name inconsistency with docstring
The method name is
boun_suang_heua_date()but the docstring title references "Ok Boun Suang Huea". Consider aligning these for consistency.Either modify the method name to match the docstring or update the docstring to match the method name.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/calendars/thai.py(23 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
holidays/calendars/thai.py
270-270: Blank line contains whitespace
Remove whitespace from blank line
(W293)
273-273: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🔇 Additional comments (9)
holidays/calendars/thai.py (9)
24-89: Great improvements to the Thai Lunar Calendar documentationThe updates to the class docstring significantly enhance the clarity and structure of the documentation. Key improvements include:
- Clearer explanation of the Thai lunar year types and month structure
- Better organization of the holiday list with consistent formatting
- Addition of more detailed festival information like "Buddha's Cremation Day" and "Ok Boun Suang Huea"
- Improved explanation of lunar phases (waxing and waning)
These changes make the documentation much more comprehensive and educational.
281-285: Good docstring simplificationThe simplified docstring is concise and appropriate for this verification method.
289-301: Improved documentation structureThe conversion to the "Args:" and "Returns:" format improves consistency with modern documentation standards and makes the parameter and return value descriptions clearer.
314-341: Enhanced holiday documentation for Makha BuchaThe docstring improvements provide greater clarity on how the date is calculated and the variations between calendar types. The "Returns:" section now explicitly mentions what happens with invalid input years.
359-385: Clear documentation format for Visakha BuchaThe consistent use of "Args:" and "Returns:" across all methods creates a uniform documentation style that improves readability.
403-424: Fixed calculation explanation for Preah NeangkoalThe correction of "Visakha Bucha +4" in line 416 makes the calculation explanation clearer and more precise.
431-458: Enhanced docstring for Atthami Bucha (Buddha's Cremation Day)Good clarification that this is "Buddha's Cremation Day" and maintaining consistency with the documentation pattern.
476-500: Improved explanation for Asarnha Bucha calculationThe rephrasing of the calculation explanation makes it easier to understand how the delta days are determined for different year types.
513-536: Clear documentation for Khao PhansaThe docstring now has a consistent format and better explains the relation to Asarnha Bucha (+1 day).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/calendars/thai.py(23 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
holidays/calendars/thai.py
269-269: SyntaxError: unindent does not match any outer indentation level
280-280: SyntaxError: unindent does not match any outer indentation level
281-281: SyntaxError: Expected class, function definition or async function definition after decorator
🪛 GitHub Actions: CI/CD
holidays/calendars/thai.py
[error] 269-269: IndentationError: unindent does not match any outer indentation level
[error] 269-269: SyntaxError: unindent does not match any outer indentation level
[error] 281-281: SyntaxError: Expected class, function definition or async function definition after decorator
[error] 753-753: SyntaxError: Expected dedent, found end of file
[error] 269-269: mypy: error: unindent does not match any outer indentation level
🔇 Additional comments (15)
holidays/calendars/thai.py (15)
23-103: Documentation significantly improved and modernizedThe docstring for
_ThaiLunisolarclass has been thoroughly improved with:
- Clearer explanation of the Thai Lunar Calendar types (Pakatimat, Athikawan, Athikamat)
- Better details on month structures and waxing/waning phases
- Comprehensive list of implemented holiday methods with their lunar calendar references
- Additional holiday entries like "Buddha's Cremation Day" and "Ok Boun Suang Huea"
These changes align well with the PR objectives to enhance documentation quality.
280-286: Documentation improvement in progressThe docstring for
__verify_calendarhas been simplified, which is good for a utility method. However, this method may be part of the syntax error chain due to the previous indentation issue.Once the previous indentation error is fixed, please verify this method works correctly.
🧰 Tools
🪛 Ruff (0.8.2)
280-280: SyntaxError: unindent does not match any outer indentation level
281-281: SyntaxError: Expected class, function definition or async function definition after decorator
🪛 GitHub Actions: CI/CD
[error] 281-281: SyntaxError: Expected class, function definition or async function definition after decorator
290-302: Documentation format updated to modern standardsThe docstring has been updated to use "Args:" and "Returns:" format instead of the older
:paramand:returnstyle, which aligns with modern Python documentation practices.
316-342: Documentation improved for clarity and consistencyThe docstring for
makha_bucha_datehas been improved with:
- Better explanation of calculation logic
- Clarified formatting with modern "Args:" and "Returns:" style
- More detailed parameter descriptions
This change is consistent with the PR's goal of enhancing documentation.
361-386: Documentation standardizedThe docstring for
visakha_bucha_datefollows the same improved pattern as other methods, with standardized "Args:" and "Returns:" sections and clearer explanations.
405-425: Documentation clarified for Preah Neangkoal methodThe explanation of the calculation method has been improved for clarity, particularly the line:
"Or as in simpler terms: 'Visakha Bucha' (Khmer Lunar Calendar) +4" which helps readers quickly understand the relationship between these dates.
433-459: Documentation enhanced for Atthami BuchaThe docstring for
atthami_bucha_datehas been updated with consistent formatting and better explanations. Nice addition of the alternative name "Buddha's Cremation Day" for better context.
478-501: Documentation improved with clearer calculation explanationThe docstring for
asarnha_bucha_datehas been updated with a better explanation of the calculation process and standardized formatting.
515-537: Documentation updated for Khao PhansaThe docstring for
khao_phansa_datehas been standardized to match the documentation style of other methods, improving overall consistency.
551-571: Documentation standardized for Boun Haw Khao PadapdinThe documentation has been updated to follow the same improved pattern as other methods, with standardized "Args:" and "Returns:" sections.
585-605: Documentation enhanced with clarificationThe docstring for
boun_haw_khao_salark_datehas been updated with improved parameter descriptions and return value explanation.
619-639: Documentation improved for Pchum Ben date calculationThe docstring has been updated with better formatting and clearer explanation of the calculation logic.
653-673: Documentation enhanced for Ok PhansaThe docstring for
ok_phansa_datehas been updated to follow the modern documentation style with clear "Args:" and "Returns:" sections.
687-707: Fixed name and updated documentation for Boun Suang HeuaThe method name in the docstring has been corrected from
boun_suang_heua_datetoOk Boun Suang Hueawith additional clarification of its alternative name.
721-741: Documentation standardized for Loy KrathongThe docstring for
loy_krathong_datehas been updated with the same improved pattern as other methods, maintaining consistency throughout the class.
|
@arkid15r I think the coderabbitai is broken somehow since all it replies are simply |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this isn't currently shown in the published documentation, it's definitely important to bring it to a single style 👍
|
For testing I used ===================================================================
diff --git a/docs/api.md b/docs/api.md
--- a/docs/api.md (revision 6b41e1c6dd31ea959cc194b739483f7e7f918f63)
+++ b/docs/api.md (date 1743962872888)
@@ -3,3 +3,4 @@
::: holidays.utils
::: holidays.holiday_base
::: holidays.ical
+::: holidays.calendars.thai
===================================================================
diff --git a/mkdocs.yml b/mkdocs.yml
--- a/mkdocs.yml (revision 6b41e1c6dd31ea959cc194b739483f7e7f918f63)
+++ b/mkdocs.yml (date 1743962872894)
@@ -18,7 +18,6 @@
docstring_style: google
inherited_members: false
filters:
- - '!^_'
- '!CountryHoliday'
members_order: source
merge_init_into_class: true |
|
@KJhellico I tried adding them to Edit: I forgot to edit mkdocs.yml, got it to appear properly now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
holidays/calendars/thai.py (1)
704-724: 🧹 Nitpick (assertive)Method name and function name mismatch.
The method is named
boun_suang_heua_datebut the docstring says "Calculate the estimated Gregorian date of Ok Boun Suang Huea." This slight inconsistency could be confusing.- """Calculate the estimated Gregorian date of Ok Boun Suang Huea. + """Calculate the estimated Gregorian date of Boun Suang Heua.
♻️ Duplicate comments (1)
holidays/calendars/thai.py (1)
51-106: 🧹 Nitpick (assertive)Consider using consistent formatting for holiday method descriptions.
The holiday method descriptions in the docstring use a consistent pattern, but based on previous review comments, they may render as single paragraphs rather than properly formatted list items in the documentation output.
Consider adding an empty line after each holiday name line for better formatting in rendered documentation:
- Magha Puja / Makha Bucha / Meak Bochea: + 15th Waxing Day (Full Moon) of Month 3 (On Month 4 for Athikamat Years). KHMER_CALENDAR always falls on Month 3.Apply similar changes to other holiday descriptions for consistent rendering.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/calendars/thai.py(23 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/calendars/thai.py (1)
Learnt from: PPsyrius
PR: vacanza/holidays#2439
File: holidays/calendars/thai.py:0-0
Timestamp: 2025-04-06T17:44:24.598Z
Learning: When making code suggestions, only include the specific lines that need to be changed in the committable suggestion part, without surrounding unrelated code.
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test build on windows-latest
🔇 Additional comments (6)
holidays/calendars/thai.py (6)
24-120: Improved documentation is well structured and comprehensive.The updated docstring provides excellent context about the Thai Lunar Calendar with clear explanations of the calendar types, month structure, and implemented holiday methods. The organization into logical sections makes it easier to understand the complex calendar system.
286-294: Documentation formatting is consistent with modern standards.The updated docstring correctly uses "Args:" and "Returns:" format, which aligns with modern Python documentation standards. This improves readability and makes the interface clearer.
307-319: Good clarification of return value behavior.The improved docstring clearly explains when the method returns None (when the year is outside the working scope), which is important for error handling in dependent code.
333-359: Clear documentation of algorithm logic with supporting detail.The documentation nicely explains both the holiday's alternative names and the calculation logic, which is valuable for maintainers who need to understand the implementation details.
422-442: Corrected calculation explanation.The updated docstring improves the explanation of the calculation for Preah Neangkoal, particularly clarifying that it uses the Khmer Lunar Calendar for calculations.
115-119:✅ Verification successful
Code example might need proper indentation for documentation rendering.
The code example appears to be formatted correctly in the source, but based on previous comments, there might be issues with how this renders in documentation.
🏁 Script executed:
#!/bin/bash # Check how code examples appear in the rendered documentation fd -t f -e md -e yml "\.md$|mkdocs\.yml$" . | xargs grep -l "ThaiLunisolar" | xargs catLength of output: 72943
Documentation Rendering of ThaiLunisolar Example Verified
After checking the rendered documentation output, the ThaiLunisolar example appears correctly indented and formatted. No indentation issues were found in the final rendered docs, so no changes are required here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/calendars/thai.py(23 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/calendars/thai.py (1)
Learnt from: PPsyrius
PR: vacanza/holidays#2439
File: holidays/calendars/thai.py:0-0
Timestamp: 2025-04-06T17:44:24.598Z
Learning: When making code suggestions, only include the specific lines that need to be changed in the committable suggestion part, without surrounding unrelated code.
🔇 Additional comments (15)
holidays/calendars/thai.py (15)
287-295: Documentation style modernizedThe docstring follows the modern "Args:" and "Returns:" format, which is more consistent with the rest of the codebase and current Python best practices.
300-300: Concise docstring improvementThe simplified docstring is clearer and more direct.
308-320: Improved method documentationThe updated docstring provides clearer information on the method's purpose and return values, following the modern documentation format.
334-360: Enhanced holiday calculation documentationThe docstring clarifies the calculation process for Makha Bucha and includes proper format for parameter and return value documentation.
379-404: Improved Visakha Bucha documentationThe updated docstring maintains clarity and follows the modern documentation style with "Args:" and "Returns:" sections.
423-443: Fixed formula description in Preah Neangkoal documentationThe corrected documentation accurately describes this as the "4th Waning Day" which matches the calculation.
451-477: Buddha's Cremation Day documentation updatedClear documentation of the Atthami Bucha calculation with proper formatting for parameters and return values.
496-519: Improved Asarnha Bucha documentationThe docstring contains more accurate information about this holiday and properly follows the modern documentation format.
533-555: Enhanced Khao Phansa documentationThe updated docstring provides clearer information and follows the modern documentation style.
569-589: Updated Boun Haw Khao Padapdin documentationDocumentation follows the established format and provides clearer descriptions.
603-623: Improved Boun Haw Khao Salark documentationThe docstring follows the modern format and provides a clear description of this holiday.
637-657: Enhanced Pchum Ben documentationThe documentation follows the established format and provides clear descriptions.
671-691: Updated Ok Phansa documentationThe documentation clearly describes this holiday and follows the modern Python docstring format.
705-725: Corrected method name in Boun Suang Heua documentationThe docstring now accurately reflects the correct holiday name and provides clearer information about the calculation.
739-759: Improved Loy Krathong documentationThe updated documentation follows the established format and provides clearer information.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proposed change
_ThaiLunisolarimplementation.Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green