Skip to content
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

implemented subdiv #608

Merged
merged 4 commits into from
Feb 3, 2022
Merged

implemented subdiv #608

merged 4 commits into from
Feb 3, 2022

Conversation

mborsetti
Copy link
Contributor

Implemented backward-compatible subdiv as per #597, #374 and addressing #548.

A few notes:

  • Code is documented and type-hinted.
  • Documentation is updated.
  • prov and state are deprecated but still work.
  • If the HolidayBase class is instantiated with a subdiv argument that is not supported, a NotImplementedError is raised.
  • If an instance is instantiated using the holidays.country_holidays('XX') method and the argument has a country code that is not supported, a NotImplementedError is raised (note: it used to raise KeyError, which is an incorrect Exception in this instance).
  • The utility function list_supported_countries now returns a dict with all the ISO 3166-1 Alpha-2 country codes of the countries supported as keys and a list of that country's supported subdivision codes as values. This is backward-compatible with existing code that iterates over countries.
  • Each country class has a new subdivisions variable listing all the subdivisions its supports. It replaces PROVINCES, which was initialized quite inconsistently (fixed).
  • Each country class has a new _deprecated_subdivisions variable listing all deprecated and/or aliases of subdivisions that are supported. This variable is private and used only to avoid triggering NotImplementedError.
  • Isle of Man has its own ISO country code of "IM" and was dutifully extracted from UnitedKingdom while maintaining backward compatibility.
  • PortugalExt was collapsed into a subdivision of Portugal while maintaining backward compatibility.
  • New Zealand was using the incorrect name and ISO subdivision code for "West Coast", which was fixed while maintaining backward compatibility.

Open issues discovered:

  • United Kingdom of Great Britain and Northern Ireland (GB) currently has "UK" as a subdivision, but the country only has four subdivisions: England, Northern Ireland, Scotland, and Wales. This needs to be to be better documented and/or cleaned up; unclear as to what "UK" is in this context.
  • The legacy code for adding HolidayBases together does some funky things. For example, adding Italy Catania (IT, CA) with USA Mississippi (US, MS) and USA Michigan (US, MI) yields country=["IT", "US"] and subdiv=["CA", "MS", "MI"], which is very ambiguous. Is it California and Messina and Milano, or Catania, Mississippi and Milano, or ... you get the picture.

@dr-prodigy
Copy link
Collaborator

Wow! This is a pretty huge + fantastic work Mike @mborsetti ! 👍
I'm still playing around with it to check the last things: if everything is fine as it seems, I'll be merging it very soon.

..I'll be back, thx a lot! :-)

@mborsetti mborsetti mentioned this pull request Feb 1, 2022
@dr-prodigy dr-prodigy merged commit 18f0263 into vacanza:beta Feb 3, 2022
@dr-prodigy
Copy link
Collaborator

Merged now! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants