Update US holidays: add federal holidays as GOVERNMENT category #2297
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2297 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 201 201
Lines 12378 12416 +38
Branches 1777 1789 +12
=========================================
+ Hits 12378 12416 +38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
WalkthroughThis pull request updates holiday classification and naming conventions across documentation, Python modules, and JSON snapshot files. In the README file, country holiday designations are revised—replacing "UNOFFICIAL" with "GOVERNMENT, UNOFFICIAL" for several regions and removing the designation for Uruguay. Several modules in the holidays/countries directory now import a new GOVERNMENT constant, update their supported_categories, and add a new Changes
Suggested reviewers
✨ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
snapshots/countries/US_AK.json (1)
1-1: 🧹 Nitpick (assertive)Future Schema Consideration for GOVERNMENT Category
While this JSON file focuses on holiday names and dates, the PR objective also mentions introducing a GOVERNMENT category for federal holidays. If the long‑term goal is to support filtering or categorization by holiday type, consider extending the JSON schema (perhaps by adding an extra field) to capture this information.snapshots/countries/US_AL.json (1)
1-1741: 🧹 Nitpick (assertive)Overall Consistency and Data Integrity Check
The updated JSON snapshot now consistently applies the new naming conventions (including the renaming of Thanksgiving, appended labels for Washington’s Birthday, adjusted Columbus Day descriptors, and the inclusion of Juneteenth). The structure remains intact, and these changes appear to propagate the intended federal holiday updates. Double-check that these changes are reflected similarly in related modules and documentation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
README.rst(6 hunks)holidays/countries/american_samoa.py(2 hunks)holidays/countries/guam.py(2 hunks)holidays/countries/northern_mariana_islands.py(2 hunks)holidays/countries/puerto_rico.py(2 hunks)holidays/countries/united_states.py(13 hunks)holidays/countries/united_states_minor_outlying_islands.py(2 hunks)holidays/countries/united_states_virgin_islands.py(2 hunks)snapshots/countries/AS_COMMON.json(101 hunks)snapshots/countries/GU_COMMON.json(101 hunks)snapshots/countries/MP_COMMON.json(100 hunks)snapshots/countries/PR_COMMON.json(66 hunks)snapshots/countries/UM_COMMON.json(50 hunks)snapshots/countries/US_AK.json(71 hunks)snapshots/countries/US_AL.json(36 hunks)snapshots/countries/US_AR.json(3 hunks)snapshots/countries/US_AS.json(101 hunks)snapshots/countries/US_AZ.json(79 hunks)snapshots/countries/US_CA.json(42 hunks)
🔇 Additional comments (63)
snapshots/countries/UM_COMMON.json (1)
1-1413: Uniform Holiday Naming ConventionsThis snapshot file has been updated extensively to standardize holiday names across decades. In particular:
Thanksgiving Updates: Every occurrence that previously used “Thanksgiving” has now been uniformly set to “Thanksgiving Day” (for example, the updates seen on lines 13, 25, 36, 48, 60, 73, 87, 98, 109, 121, etc.). This update ensures that users and downstream services will consistently refer to the holiday using the new naming standard.
Martin Luther King, Jr. Day Enhancement: The holiday originally labeled “Martin Luther King Jr. Day” has been enhanced to include the commemorative prefix—now reading “Birthday of Martin Luther King, Jr.; Martin Luther King Jr. Day” (as shown in many entries starting from line 454, 467, 481, 496, 510, etc.). This change not only improves clarity but also aligns with the updated localization standards.
The JSON structure remains valid and well‐formatted. These comprehensive changes clearly support the PR objective of improved holiday categorization and naming conventions. Great work on applying the updates consistently across the dataset!
snapshots/countries/MP_COMMON.json (3)
18-18: Consistent naming standardization for Thanksgiving.The renaming from "Thanksgiving" to "Thanksgiving Day" brings consistency to holiday naming conventions across all years in the file.
Also applies to: 36-36, 53-53, 70-70, 87-87, 105-105, 126-126, 144-144, 161-161, 178-178, 194-194, 214-214, 233-233, 251-251, 269-269, 286-286, 304-304, 323-323, 340-340, 357-357, 374-374, 391-391, 410-410, 428-428, 446-446, 463-463, 480-480, 498-498, 517-517, 536-536, 553-553, 570-570, 587-587, 605-605, 622-622, 640-640, 662-662, 680-680, 697-697, 717-717, 735-735, 756-756, 775-775, 793-793, 812-812, 833-833, 851-851, 869-869, 887-887, 905-905, 926-926, 946-946, 965-965, 983-983, 1001-1001, 1020-1020, 1041-1041, 1061-1061, 1079-1079, 1097-1097, 1116-1116, 1135-1135, 1158-1158, 1177-1177, 1195-1195, 1213-1213, 1231-1231, 1252-1252, 1273-1273, 1292-1292, 1312-1312, 1332-1332, 1354-1354, 1376-1376, 1396-1396, 1415-1415, 1435-1435, 1455-1455, 1478-1478, 1499-1499, 1520-1520, 1539-1539, 1560-1560, 1583-1583, 1604-1604, 1624-1624, 1643-1643, 1662-1662, 1683-1683, 1704-1704, 1728-1728, 1748-1748, 1767-1767, 1786-1786, 1810-1810, 1828-1828, 1849-1849, 1869-1869, 1890-1890, 1910-1910, 1932-1932
649-649: Enhanced naming for MLK Jr. Day follows federal designation.The expanded holiday name "Birthday of Martin Luther King, Jr.; Martin Luther King Jr. Day" aligns with official federal government naming conventions, providing both the formal and common names.
Also applies to: 666-666, 684-684, 703-703, 721-721, 742-742, 761-761, 779-779, 799-799, 818-818, 837-837, 856-856, 873-873, 891-891, 911-911, 930-930, 951-951, 970-970, 987-987, 1007-1007, 1026-1026, 1045-1045, 1066-1066, 1083-1083, 1101-1101, 1122-1122, 1141-1141, 1163-1163, 1182-1182, 1199-1199, 1217-1217, 1237-1237, 1256-1256, 1278-1278, 1297-1297, 1316-1316, 1338-1338, 1360-1360, 1380-1380, 1401-1401, 1419-1419, 1439-1439, 1461-1461, 1482-1482, 1504-1504, 1525-1525, 1543-1543, 1566-1566, 1587-1587, 1608-1608, 1629-1629, 1647-1647, 1666-1666, 1689-1689, 1710-1710, 1733-1733, 1754-1754, 1771-1771, 1790-1790, 1812-1812, 1832-1832, 1854-1854, 1874-1874, 1894-1894, 1916-1916
387-387: Consistent dual categorization for Columbus Day.All entries for "Columbus Day" correctly maintain the dual categorization as "Columbus Day; Commonwealth Cultural Day", preserving local cultural significance alongside the federal holiday name.
Also applies to: 405-405, 423-423, 442-442, 471-471, 494-494, 511-511, 530-530, 549-549, 566-566, 583-583, 601-601, 620-620, 658-658, 676-676, 693-693, 711-711, 731-731, 752-752, 771-771, 789-789, 801-801, 865-865, 883-883, 901-901, 920-920, 940-940, 962-962, 979-979, 993-993, 1013-1013, 1035-1035, 1055-1055, 1075-1075, 1093-1093, 1111-1111, 1131-1131, 1151-1151, 1173-1173, 1191-1191, 1209-1209, 1226-1226, 1246-1246, 1266-1266, 1288-1288, 1307-1307, 1328-1328, 1349-1349, 1370-1370, 1391-1391, 1411-1411, 1430-1430, 1451-1451, 1471-1471, 1493-1493, 1515-1515, 1535-1535, 1555-1555, 1577-1577, 1597-1597, 1618-1618, 1639-1639, 1658-1658, 1678-1678, 1700-1700, 1721-1721, 1744-1744, 1763-1763, 1782-1782, 1801-1801, 1822-1822, 1842-1842, 1865-1865, 1885-1885, 1906-1906, 1927-1927
holidays/countries/guam.py (3)
13-13: Added GOVERNMENT constant import to support new holiday category.Importing the GOVERNMENT constant from holidays.constants is necessary to support the new government holiday category functionality.
23-23: Updated supported_categories to include GOVERNMENT.Adding GOVERNMENT to the supported_categories tuple enables proper filtering and categorization of government holidays for Guam.
30-33: Added method to populate government holidays for Guam.The new _populate_government_holidays method correctly sets the subdiv to "GU" and calls the parent method, maintaining consistency with the existing pattern for public and unofficial holidays.
holidays/countries/puerto_rico.py (3)
13-13: Added GOVERNMENT constant import for Puerto Rico.Importing the GOVERNMENT constant is necessary to support the categorization of federal holidays under the government category.
23-23: Updated supported_categories to include GOVERNMENT.Adding GOVERNMENT to the supported_categories expands holiday classification options for Puerto Rico, ensuring proper categorization of federal holidays.
30-33: Added method to populate government holidays for Puerto Rico.The new _populate_government_holidays method follows the established pattern by setting the subdiv to "PR" and calling the parent implementation, maintaining code consistency.
holidays/countries/northern_mariana_islands.py (3)
13-13: Added GOVERNMENT constant import for Northern Mariana Islands.The addition of the GOVERNMENT import is necessary to support the government holiday category for MP.
23-23: Updated supported_categories to include GOVERNMENT.Adding GOVERNMENT to the supported_categories enables proper filtering and categorization of government holidays for Northern Mariana Islands.
30-33: Added method to populate government holidays for Northern Mariana Islands.The new _populate_government_holidays method correctly sets the subdiv to "MP" before calling the parent implementation, maintaining consistent approach with other US territories.
holidays/countries/united_states_minor_outlying_islands.py (3)
13-13: Addition of GOVERNMENT constant is appropriate.This change correctly adds the GOVERNMENT constant from holidays.constants, aligning with the PR's objective to add federal holidays under the GOVERNMENT category.
23-23: Category tuple correctly updated to include GOVERNMENT.The supported_categories tuple has been properly expanded to include GOVERNMENT along with the existing PUBLIC and UNOFFICIAL categories.
30-33: Appropriate implementation of _populate_government_holidays method.This method properly sets the subdiv attribute to "UM" and calls the parent class method, establishing the inheritance pattern used in other similar classes.
holidays/countries/american_samoa.py (3)
13-13: Addition of GOVERNMENT constant is consistent with other changes.This import change matches the pattern applied to other files in this PR, correctly importing the GOVERNMENT constant.
23-23: Category tuple updated consistently.The supported_categories tuple now includes GOVERNMENT alongside PUBLIC and UNOFFICIAL, maintaining consistency with other US territory implementations.
30-33: Implementation follows consistent pattern.The new _populate_government_holidays method uses the same implementation approach as the other population methods, setting the subdiv to "AS" before calling the parent method.
snapshots/countries/US_AR.json (4)
6-6: Properly standardized Thanksgiving to Thanksgiving Day.The PR's objective to rename Thanksgiving to Thanksgiving Day has been consistently applied across all years in the file. This standardization improves naming consistency.
Also applies to: 14-14, 27-27, 40-40, 53-53, 66-66, 80-80, 96-96, 108-108, 120-120, 133-133, 146-146, 161-161, 174-174, 186-186, 200-200, 213-213, 227-227, 242-242, 255-255, 267-267, 280-280, 294-294, 310-310, 323-323, 336-336, 352-352, 379-379, 409-409, 423-423, 437-437, 451-451, 466-466, 483-483, 496-496, 510-510, 525-525, 540-540, 557-557, 572-572, 586-586, 602-602, 617-617, 633-633, 649-649, 665-665, 679-679, 694-694, 709-709, 727-727, 742-742, 756-756, 770-770, 786-786, 802-802, 834-834, 849-849, 865-865, 895-895, 913-913, 927-927, 941-941, 1003-1003, 1017-1017, 1033-1033, 1050-1050, 1068-1068, 1086-1086, 1102-1102, 1117-1117, 1133-1133, 1150-1150, 1169-1169, 1185-1185, 1200-1200, 1215-1215, 1233-1233, 1251-1251, 1269-1269, 1285-1285, 1301-1301, 1317-1317, 1334-1334, 1352-1352, 1371-1371, 1386-1386, 1401-1401, 1417-1417, 1434-1434, 1452-1452, 1468-1468, 1483-1483, 1500-1500, 1517-1517, 1535-1535
6-6: Addition of Washington's Birthday as a federal holiday designation.The addition of "Washington's Birthday" to dates already marked as "George Washington's Birthday and Daisy Gatson Bates Day" properly identifies these as federal holidays under the GOVERNMENT category while preserving the state-specific name.
Also applies to: 19-19, 32-32, 45-45, 58-58, 73-73, 87-87, 101-101, 113-113, 125-125, 138-138, 153-153, 166-166, 179-179, 191-191, 205-205, 220-220, 234-234, 247-247, 260-260, 272-272, 285-285, 301-301, 315-315, 328-328, 342-342, 354-354, 371-371, 386-386, 400-400, 414-414, 428-428, 442-442, 458-458, 473-473, 488-488, 502-502, 516-516, 531-531, 548-548, 563-563, 578-578, 592-592, 608-608, 625-625, 641-641, 657-657, 671-671, 685-685, 700-700, 717-717, 733-733, 748-748, 762-762, 776-776, 794-794, 810-810, 825-825, 840-840, 857-857, 870-870, 887-887, 902-902, 918-918, 933-933, 947-947, 962-962, 989-989, 1009-1009, 1023-1023, 1039-1039, 1058-1058, 1076-1076, 1092-1092, 1108-1108, 1123-1123, 1139-1139, 1158-1158, 1175-1175, 1191-1191, 1206-1206, 1221-1221, 1241-1241, 1259-1259, 1275-1275, 1291-1291, 1307-1307, 1323-1323, 1342-1342, 1360-1360, 1377-1377, 1392-1392, 1407-1407, 1423-1423, 1442-1442, 1458-1458, 1474-1474, 1489-1489, 1506-1506, 1525-1525
291-292: Columbus Day consistently marked.The entry for Columbus Day on the second Monday of October is consistently renamed across all years. This change is part of the standardization of federal holiday names.
Also applies to: 306-307, 320-321, 334-335, 346-347, 361-362, 377-378, 406-407, 430-431, 446-447, 463-464, 506-507, 522-523, 536-537, 568-569, 598-599, 614-615, 630-631, 646-647, 691-692, 706-707, 722-723, 738-739, 753-754, 767-768, 781-782, 797-798, 813-814, 829-830, 845-846, 876-877, 906-907
499-500: MLK Day consistently identified with proper formatting.Martin Luther King Jr. Day is consistently identified with the proper formatting across all years, aligning with federal holiday naming conventions.
Also applies to: 513-514, 528-529, 545-546, 560-561, 575-576, 589-590, 605-606, 622-623, 638-639, 653-654, 668-669, 682-683, 698-699, 714-715, 730-731, 745-746, 760-761, 773-774, 790-791, 806-807, 822-823, 837-838, 853-854, 867-868, 883-884, 899-900, 916-917, 930-931, 944-945, 976-977, 991-992, 1006-1007, 1020-1021, 1036-1037, 1055-1056, 1073-1074, 1089-1090, 1105-1106, 1120-1121, 1136-1137, 1155-1156, 1172-1173, 1188-1189, 1203-1204, 1218-1219, 1238-1239, 1256-1257, 1272-1273, 1288-1289, 1304-1305, 1320-1321, 1339-1340, 1357-1358, 1374-1375, 1389-1390, 1404-1405, 1420-1421, 1439-1440, 1455-1456, 1471-1472, 1486-1487, 1503-1504, 1522-1523
README.rst (2)
208-208: Updated category designations are consistent with code changes.The README now accurately reflects the addition of GOVERNMENT category to American Samoa, Guam, Northern Mariana Islands, Puerto Rico, US Minor Outlying Islands, US, and US Virgin Islands. This change aligns with the code modifications in the respective country modules.
Also applies to: 488-488, 723-723, 783-783, 933-933, 938-938, 978-978
948-948:✅ Verification successful
Updated Uruguay entry by removing UNOFFICIAL designation.
The UNOFFICIAL designation has been removed from Uruguay, but no corresponding code change was identified in the files provided for review.
🏁 Script executed:
#!/bin/bash # Check Uruguay holidays file to confirm category changes grep -r "supported_categories" --include="*uruguay.py" ./holidays/Length of output: 140
Verified Uruguay Category Update:
The latest grep output confirms that Uruguay’s holiday file now correctly showssupported_categories = (BANK, PUBLIC), reflecting the removal of the UNOFFICIAL designation. Please update the README snippet if it still appears outdated.holidays/countries/united_states_virgin_islands.py (3)
13-13: Import added correctly to support new holiday category.Adding the GOVERNMENT constant from holidays.constants is appropriate and necessary to implement the new category for federal holidays.
23-23: Category update correctly implements PR objective.The addition of GOVERNMENT to the supported_categories tuple is properly implemented and aligns with the PR goal of introducing federal holidays under the GOVERNMENT category.
30-32: Implementation follows established pattern.The new _populate_government_holidays method follows the same pattern as the existing methods for public and unofficial holidays. The method properly sets the subdiv attribute to "VI" before calling the superclass method.
snapshots/countries/GU_COMMON.json (3)
16-16: Standardized holiday name from "Thanksgiving" to "Thanksgiving Day".Consistently applied name standardization across all years from 1950 through 2050, which aligns with the PR objectives to rename Thanksgiving to Thanksgiving Day throughout the codebase.
Also applies to: 32-32, 47-47, 63-63, 79-79, 96-96, 114-114, 129-129, 144-144, 160-160, 175-175, 193-193, 209-209, 224-224, 240-240, 256-256, 273-273, 291-291, 306-306, 321-321, 338-338, 356-356, 375-375, 392-392, 409-409, 426-426, 444-444, 463-463, 483-483, 501-501, 518-518, 536-536, 554-554, 573-573, 593-593, 610-610, 628-628, 647-647, 665-665, 686-686, 705-705, 723-723, 742-742, 761-761, 781-781, 802-802, 820-820, 838-838, 857-857, 876-876, 897-897, 916-916, 934-934, 952-952, 971-971, 991-991, 1012-1012, 1031-1031, 1049-1049, 1068-1068, 1087-1087, 1107-1107, 1128-1128, 1146-1146, 1164-1164, 1183-1183, 1201-1201, 1222-1222, 1241-1241, 1259-1259, 1278-1278, 1299-1299, 1321-1321, 1343-1343, 1362-1362, 1381-1381, 1401-1401, 1422-1422, 1444-1444, 1464-1464, 1483-1483, 1502-1502, 1523-1523, 1545-1545, 1567-1567, 1587-1587, 1606-1606, 1626-1626, 1647-1647, 1669-1669, 1691-1691, 1710-1710, 1729-1729, 1749-1749, 1769-1769, 1791-1791, 1811-1811, 1830-1830, 1850-1850, 1871-1871, 1893-1893
614-614: Expanded MLK Day name to include formal designation.Consistently updated holiday name from "Martin Luther King Jr. Day" to "Birthday of Martin Luther King, Jr.; Martin Luther King Jr. Day" for all occurrences from 1986 through 2050. This standardization aligns with official federal holiday naming conventions.
Also applies to: 632-632, 651-651, 671-671, 690-690, 709-709, 727-727, 746-746, 767-767, 787-787, 806-806, 824-824, 842-842, 861-861, 882-882, 901-901, 920-920, 938-938, 956-956, 977-977, 997-997, 1016-1016, 1035-1035, 1053-1053, 1072-1072, 1093-1093, 1113-1113, 1132-1132, 1150-1150, 1168-1168, 1187-1187, 1207-1207, 1226-1226, 1245-1245, 1263-1263, 1282-1282, 1305-1305, 1327-1327, 1347-1347, 1366-1366, 1385-1385, 1405-1405, 1428-1428, 1448-1448, 1468-1468, 1487-1487, 1506-1506, 1529-1529, 1551-1551, 1571-1571, 1591-1591, 1610-1610, 1630-1630, 1653-1653, 1675-1675, 1695-1695, 1714-1714, 1733-1733, 1753-1753, 1775-1775, 1795-1795, 1815-1815, 1834-1834, 1854-1854, 1877-1877
352-352: Columbus Day entries retained consistently.Columbus Day entries have been maintained across all years, keeping consistent holiday data for historical accuracy while preserving the original holiday name.
Also applies to: 371-371, 388-388, 405-405, 422-422, 440-440, 459-459, 478-478, 496-496, 514-514, 532-532, 550-550, 569-569, 588-588, 606-606, 624-624, 643-643, 661-661, 681-681, 700-700, 719-719, 738-738, 757-757, 777-777, 797-797, 816-816, 834-834, 853-853, 872-872, 892-892, 911-911, 930-930, 948-948, 967-967, 987-987, 1007-1007, 1026-1026, 1045-1045, 1064-1064, 1083-1083, 1103-1103, 1123-1123, 1142-1142, 1160-1160, 1179-1179, 1197-1197, 1217-1217, 1236-1236, 1255-1255, 1274-1274, 1295-1295, 1317-1317, 1338-1338, 1358-1358, 1377-1377, 1397-1397, 1418-1418, 1439-1439, 1459-1459, 1479-1479, 1498-1498, 1519-1519, 1541-1541, 1562-1562, 1582-1582, 1602-1602, 1622-1622, 1643-1643, 1665-1665, 1686-1686, 1706-1706, 1725-1725, 1745-1745, 1765-1765, 1786-1786, 1806-1806, 1826-1826, 1846-1846, 1867-1867, 1889-1889
snapshots/countries/US_AS.json (2)
16-16: Renaming “Thanksgiving” to “Thanksgiving Day” looks consistent throughout.
All entries align with federal naming conventions. Nicely done.Also applies to: 31-31, 44-44, 59-59, 75-75, 92-92, 109-109, 123-123, 136-136, 151-151, 166-166, 183-183, 198-198, 212-212, 227-227, 243-243, 260-260, 275-275, 291-291, 304-304, 319-319, 335-335, 351-351, 365-365, 379-379, 393-393, 409-409, 426-426, 443-443, 458-458, 472-472, 487-487, 503-503, 522-522, 540-540, 555-555, 571-571, 588-588, 611-611, 626-626, 643-643, 659-659, 676-676, 694-694, 706-706, 719-719, 737-737, 753-753, 769-769, 786-786, 801-801, 824-824, 841-841, 857-857, 873-873, 893-893, 913-913, 931-931, 948-948, 964-964, 981-981, 1001-1001, 1021-1021, 1038-1038, 1054-1054, 1070-1070, 1087-1087, 1107-1107, 1125-1125, 1142-1142, 1158-1158, 1175-1175, 1197-1197, 1219-1219, 1238-1238, 1255-1255, 1272-1272, 1290-1290, 1312-1312, 1331-1331, 1349-1349, 1366-1366, 1383-1383, 1405-1405, 1422-1422, 1438-1438, 1451-1451, 1469-1469, 1486-1486, 1504-1504, 1516-1516, 1538-1538, 1558-1558, 1575-1575, 1592-1592, 1610-1610, 1630-1630, 1651-1651, 1669-1669, 1686-1686, 1704-1704, 1724-1724, 1746-1746
558-558: “Birthday of Martin Luther King, Jr.; Martin Luther King Jr. Day” is properly expanded.
This clarifies the holiday’s official name. Good improvement.Also applies to: 574-574, 591-591, 611-611, 629-629, 646-646, 662-662, 679-679, 699-699, 719-719, 737-737, 753-753, 769-769, 786-786, 806-806, 824-824, 841-841, 857-857, 873-873, 893-893, 913-913, 931-931, 948-948, 964-964, 981-981, 1001-1001, 1021-1021, 1038-1038, 1054-1054, 1070-1070, 1087-1087, 1107-1107, 1125-1125, 1142-1142, 1158-1158, 1175-1175, 1197-1197, 1219-1219, 1238-1238, 1255-1255, 1272-1272, 1290-1290, 1312-1312, 1331-1331, 1349-1349, 1366-1366, 1383-1383, 1405-1405, 1422-1422, 1438-1438, 1451-1451, 1469-1469, 1486-1486, 1504-1504, 1521-1521, 1543-1543, 1561-1561, 1578-1578, 1595-1595, 1613-1613, 1630-1630, 1651-1651, 1669-1669, 1686-1686, 1704-1704, 1724-1724, 1746-1746
holidays/countries/united_states.py (15)
16-16: Importing GOVERNMENT is aligned with the new holiday category.
No issues found.
38-40: Docstring: Federal holidays reference is accurate.
This clarifies the official OPM link. Nicely done.
91-91: Including GOVERNMENT in supported_categories.
Matches PR objectives of adding a GOVERNMENT category.
166-166: _populate_common with is_federal parameter.
This parameter elegantly branches out federal holiday logic. Looks correct.Also applies to: 173-185
187-188: Memorial Day logic from 1971 onward.
This date range is compliant with historical legislation.
202-206: Conditional add for Columbus Day under is_federal.
Properly guarded by the federal check. Good approach.
215-215: Renaming to “Thanksgiving Day”.
Consistent with the data file updates. Well done.Also applies to: 217-217
223-225: _populate_public_holidays delegates to _populate_common.
Stays consistent with the code’s design pattern.
351-356: Alabama’s Washington’s Birthday adjustments.
References are correct for local state naming.
383-390: Arkansas’s Washington’s Birthday merges Daisy Gatson Bates Day.
This aligns with Arkansas specifics. Verified.
523-527: Georgia’s adjusted approach for Washington’s Birthday.
The special observed rule is properly integrated.
617-622: Indiana’s updated logic for Washington’s Birthday.
Implementation is consistent with the local name references.
904-905: Puerto Rico’s Presidents’ Day condition.
The year-based check is correct.
1047-1053: Virgin Islands and Presidents’ Day.
Correctly handles older references to Washington’s Birthday. Good detail.
1135-1138: _populate_government_holidays calls _populate_common(is_federal=True).
Cleanly unifies federal holiday logic under the GOVERNMENT category.snapshots/countries/AS_COMMON.json (1)
1-1750: Overall File Consistency and Valid JSON Structure
The JSON file maintains proper syntax and structure, and the updates have been applied consistently across all the holiday entries. The renaming changes are uniformly integrated, which will support accurate localization and future data processing.snapshots/countries/US_AK.json (4)
6-6: Adding "Washington's Birthday" for 1950
Nice work adding the "Washington's Birthday" entry on 1950‑02‑22. This update helps standardize the federal holiday naming across the dataset. Please verify once more that the date is accurate with the official holiday information.
16-16: Renaming Thanksgiving to "Thanksgiving Day"
The update renaming the Thanksgiving entry on 1950‑11‑23 to "Thanksgiving Day" is spot on. This change aligns with the updated naming conventions mentioned in the PR. Ensure that any downstream uses (such as documentation references or filtering logic) are updated accordingly.
21-1470: Consistent Federal Holiday Updates Across Years
This snapshot now shows a systematic update—from 1950 through later years—where multiple entries, such as "Washington's Birthday" and the renaming of Thanksgiving to "Thanksgiving Day," are applied uniformly. The consistency in applying these naming conventions is commendable. It would be good to double-check that these changes mirror the official holiday calendars and are reflected in other state snapshots as well.
1-1770: Overall Harmonization of Holiday Naming
Overall, the changes in this snapshot do a great job of harmonizing the holiday names—adding the "Washington's Birthday" mark and renaming Thanksgiving to "Thanksgiving Day" as intended by the PR objectives. Confirm that any associated documentation and tests are updated to reflect these changes.snapshots/countries/US_CA.json (4)
1-1760: Overall Consistency and Format CheckThe JSON snapshot remains well-formatted and valid. Across the file, you’ve uniformly updated the holiday names—most notably, introducing "Washington's Birthday" for February 22 and renaming "Thanksgiving" to "Thanksgiving Day" for the applicable dates. These changes appear to be applied consistently over a wide date range. Please double-check that these naming updates dovetail with the updated federal holiday categorization (i.e. as GOVERNMENT holidays) elsewhere in the codebase or documentation.
6-7: Washington's Birthday Update VerificationEntries for February 22 (for example, on "1950-02-22", "1951-02-22", "1952-02-22", etc.) have been updated to "Washington's Birthday" consistently. This standardization is critical for federal holiday clarity and appears to be correctly applied.
Also applies to: 19-20, 32-33, 45-46, 58-59, 73-74, 87-88, 101-102
14-14: Thanksgiving Day Renaming ConsistencyHoliday entries originally labeled "Thanksgiving" have now been updated to "Thanksgiving Day" (e.g. "1950-11-23", "1951-11-22", "1952-11-27", etc.). This ensures the naming is uniform across the snapshot, which improves clarity and consistency.
Also applies to: 27-27, 40-40, 53-53, 66-66, 108-108, 120-120
284-292: Presidents' Day Entry NoteOne of the entries now reads “Presidents' Day; Washington's Birthday” (as seen on, for instance, the 1971 entry). Please verify that merging these two holiday names into a single string is intentional and that it aligns with how other modules are expected to interpret federal holiday categorizations.
snapshots/countries/PR_COMMON.json (1)
1-1961: Consistent Holiday Naming Updates Across All YearsYour modifications standardize the holiday names throughout the JSON file in a very systematic way. In particular, all instances of "Thanksgiving" have been updated to "Thanksgiving Day" and references to the traditional "Presidents' Day" have been replaced with either "Washington's Birthday" or, in later years where historical context is preserved, as "Presidents' Day; Washington's Birthday." These changes ensure that the snapshot aligns with the updated federal holiday naming conventions outlined in the PR objectives.
The structure and JSON formatting remain intact, and the changes are applied consistently across all keys. Please verify that these modified names conform to any external guidelines, especially for years where a dual-label (e.g., "Presidents' Day; Washington's Birthday") is used.
snapshots/countries/US_AZ.json (1)
1-1560: Overall Consistency & Structure CheckThe changes throughout the file clearly update holiday naming conventions in line with the PR objectives: all instances of "Thanksgiving" have been renamed to "Thanksgiving Day," and entries for "Washington's Birthday" have been added (or updated) on the appropriate dates (e.g., “1950-02-22”, “1951-02-22”, etc.). The JSON structure remains valid and well-formatted.
A couple of points to consider:
- Verify that the historical and observed dates (for holidays like Independence Day, Veterans Day, etc.) remain correct after these mass renaming changes.
- The PR summary mentions adding federal holidays under a GOVERNMENT category. This snapshot file only includes holiday names. Please confirm that the categorization (or additional metadata) is intended to be handled elsewhere, as no explicit "GOVERNMENT" field is present in the JSON.
Overall, the renaming and insertion changes appear consistent across the dataset.
snapshots/countries/US_AL.json (4)
6-6: Consistent Update to Washington's Birthday Naming
Across many entries (e.g. on lines 6, 51, 99, 143, 175, 219, 235, 283, 312, and 327), the holiday previously labeled as
"George Washington & Thomas Jefferson's Birthday"
has been updated to include the appended text
"; Washington's Birthday".
This clear and systematic standardization improves clarity in the holiday descriptions in line with the PR’s objective.Also applies to: 51-51, 99-99, 143-143, 175-175, 219-219, 235-235, 283-283, 312-312, 327-327
16-16: Renaming Thanksgiving for Uniformity
The renaming of “Thanksgiving” to “Thanksgiving Day” (as seen for example on lines 16, 31, 46, 61, 76, and 92) is applied consistently across multiple years. This change aligns with the intended update to naming conventions and helps eliminate ambiguity.Also applies to: 31-31, 46-46, 61-61, 76-76, 92-92
1186-1187: New Federal Holiday: Juneteenth Added
The inclusion of Juneteenth—both as an observed variant and as a regular holiday (e.g. lines 1186–1187, 1207–1208, and 1264)—reflects its recent federal holiday status. This update is an excellent enhancement aligning with current holiday recognitions.Also applies to: 1207-1208, 1264-1264
571-571:❓ Verification inconclusive
Verify MLK Day Naming Update
In several records (for instance, line 571, 1006, and 1178) the description for Martin Luther King, Jr. Day now appears as:
"Birthday of Martin Luther King, Jr.; Martin Luther King, Jr & Robert E. Lee's Birthday".
Since the PR objective emphasizes retaining Martin Luther King Jr. Day without any merger, please confirm whether the inclusion of "Robert E. Lee's Birthday" is intentional. This detail may need a second look to ensure alignment with the update’s intent.Also applies to: 1006-1006, 1178-1178
MLK Day Description Clarification Needed
In the filesnapshots/countries/US_AL.json(lines 571, 1006, and 1178), the holiday record currently displays the merged description:
"Birthday of Martin Luther King, Jr.; Martin Luther King, Jr & Robert E. Lee's Birthday".Since the PR’s objective is to retain Martin Luther King, Jr. Day as a distinct celebration, please verify whether including "Robert E. Lee's Birthday" here is intentional or if it should be removed to avoid merging the two events.
# Conflicts: # snapshots/countries/US_MI.json
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/united_states.py(13 hunks)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test build on windows-latest
🔇 Additional comments (16)
holidays/countries/united_states.py (16)
16-16: Looks good adding theGOVERNMENTconstant.
No issues spotted in this import line.
38-40: Documentation update acknowledged.
No concerns with these docstring additions.
92-92: New category support is correct.
IncludingGOVERNMENTinsupported_categoriesaligns with the PR objective.
174-178: Federal MLK holiday logic looks good.
The checkif self._year >= 1986:is accurate historically.
179-186: Washington's Birthday logic is consistent.
The year thresholds for the holiday are correct.
188-189: Memorial Day shift is aligned with 1971 changes.
This follows the Uniform Monday Holiday Act. No issues.
203-207: Columbus Day for federal category is correct.
No concerns with the year check and holiday addition.
216-218: Renaming to “Thanksgiving Day.”
Matches the PR direction on holiday naming consistency.
224-230: Public holiday overlap for Memorial Day pre-1971 is maintained.
Retaining historical date coverage is sensible.
352-357: Alabama’s Washington & Jefferson addition is valid.
No issues found in the year checks or naming.
385-390: Arkansas’s naming logic looks good.
Year-based splits for Washington’s Birthday are correct.
523-527: Georgia’s offset for Washington’s Birthday noted.
The approach and rule usage are consistent with local code references.
618-623: Indiana’s Washington’s Birthday approach is aligned.
Reusing the same holiday shift pattern is fine.
912-914: Presidents’ Day for Puerto Rico is properly guarded by year check.
No concerns noticed.
1056-1061: Virgin Islands Presidents’ Day logic is correct.
Preserved the older fallback for Feb 22 if pre-1971.
1144-1147: New method for government holidays is consistent.
This is an appropriate place to setis_federal=True.
|
Caution An unexpected error occurred while opening a pull request: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
1 similar comment
|
Caution An unexpected error occurred while opening a pull request: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
holidays/countries/united_states.py (1)
624-629:⚠️ Potential issueLikely incorrect date reference for Washington’s Birthday.
Identical concern as in the GA block. Anchoring to Christmas Day may yield an erroneous date.Apply a similar fix here, anchoring directly to February.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/united_states.py(13 hunks)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test build on windows-latest
🔇 Additional comments (14)
holidays/countries/united_states.py (14)
16-16: Import constants to support the new GOVERNMENT category.
This aligns with the PR objective to distinguish federal holidays.
38-40: Helpful reference to official sources.
Links to federal holidays are clearly documented.
92-92: Expanding categories with GOVERNMENT.
Including GOVERNMENT here meets the PR requirement for new classifications.
167-173: Docstring addition looks good.
Thank you for clarifying theis_federalparameter.
180-193: Well-structured approach for federal holidays.
Leveragingis_federalto include specific holidays is clear and maintainable.
194-195: Verify classification for Memorial Day.
Memorial Day is a federal holiday. If you want it exclusive to GOVERNMENT, consider placing it underif is_federal:or confirm it applies to all categories.
209-213: Consistent federal handling of Columbus Day.
The conditional block for federal-only logic is straightforward.
222-224: Thanksgiving Day rename is consistent with PR objectives.
Renaming “Thanksgiving” to “Thanksgiving Day” aligns with the changes.
230-235: Unified logic for public holidays.
Invoking_populate_common()then applying legacy Memorial Day coverage is cohesive.
358-363: Appropriate naming for AL holidays.
Adding “George Washington & Thomas Jefferson's Birthday” is historically consistent.
391-397: AR holiday naming is correct.
“George Washington's Birthday and Daisy Gatson Bates Day” is properly reflected.
918-920: Presidents' Day for PR looks good.
This check ensures the holiday starts at the right year.
1062-1068: Proper addition of Presidents’ Day in VI.
Conditional logic is aligned with the rest of the code.
1150-1153: New method for government holidays is clear.
Design cleanly delegates federal logic to_populate_common(is_federal=True).
Proposed change
Update US holidays: add federal holidays as GOVERNMENT category.
Key points (for discussion):
subdiv=None)GOVERNMENTholidaysType of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green