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

Sphinx docs for Read the Docs #602

Merged
merged 3 commits into from
Jan 17, 2022
Merged

Sphinx docs for Read the Docs #602

merged 3 commits into from
Jan 17, 2022

Conversation

mborsetti
Copy link
Contributor

@mborsetti mborsetti commented Jan 15, 2022

@dr-prodigy Not sure why it was so complex to incorporate the commits you made today (sigh) into this branch, but finally here is a PR that can both be merged and that passes all tox tests.

Finally, I had some time to finish the Sphinx/readthedocs.io documentation. There was a lot of work and QA involved in doing it, but I think you will be very pleased with the result. The documentation is now (in my opinion) much more legible and user friendly, and finally the API is 100% documented -- inline, in IDE, and on Read the Docs.

The following changes were needed for the documentation:

  • README.rst has been simplified and now acts as the welcome page of the documentation on Read the Docs.
  • Advanced examples from README.rst have been moved to an "Additional Example" page on Read the Docs (and prettified).
  • Contribution information from README.rst has been moved to the new file CONTRIBUTING.rst, which is also imported in the documentation on Read the Docs.
  • Formalized the prioritization of the use of the ISO 3166-1 alpha-2 code as a method to instantiate a HolidayBase class.
  • Prioritized the use of CountryHoliday (now aliased to the more Python-friendly country_holidays) for documentation purposes.
  • Fully documented the API using docstrings and autodoc to pull them into Sphinx/Read the Docs.
  • So that they would not be picked in the documentation as public APIs, formalized the ChineseLuniSolar class and islamic_to_gre as being private (i.e. renamed to _ChineseLuniSolar and _islamic_to_gre), and this in turn required minor edits to a high number of country files.
  • Tox now builds the documentation as a check to ensure it builds.
  • Had to (temporarily hopefully) disable rstcheck in pre-commit due to throwing an intractable error, at least in my Windows setup. I am not too worried about it since we're testing a full documentation build in tox, but I am still highly annoyed.
  • Opportunistically fixed TestTaiwan for Taiwan (was called TestTunisia!) and the ISO 3166-1 alpha-2 country code in the country variable of Bangladesh ("BD" and not "TR"), United Kingdom ("GB" and not "UK") and of ECB/TARGET2 ("ECB" and not "EU"); I know good practice for this to be a separate PR, but got too lazy, so apologies ;)

I could not test an actual Read the Docs build, but given that a full document build passes the tox test, I have high confidence that it will build without a problem (typos and all).

Incidentally, one of the many good things about building the documentation on Sphinx/readthedocs.io is that we finally have a way to publish individual country pages (with e.g. information such as that country's subdivisions, sources of data, why certain holidays are included and others aren't, etc. etc.), which will be very useful as the project continues to grow.

Edits are obviously welcomed; you're the maintainer!

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1701876142

  • 100 of 103 (97.09%) changed or added relevant lines in 24 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 97.885%

Changes Missing Coverage Covered Lines Changed/Added Lines %
holidays/utils.py 10 11 90.91%
holidays/holiday_base.py 17 19 89.47%
Totals Coverage Status
Change from base Build 1701225143: -0.05%
Covered Lines: 5797
Relevant Lines: 5865

💛 - Coveralls

@dr-prodigy
Copy link
Collaborator

Hey there @mborsetti
this is great!
I'm sorry for your extra work.. what I believe is: since you started working on this PR (quite a long time ago now!), I've been merging a lot of new features / fixes on my beta branch (I also released a new version to master / PyPi, but that's pretty uninfluent).
In that meantime, you have probably never merged changes / rebased from my repo (always keeping on working on your copy), so when you finally released this PR, you got them all back (messing up what was already complete on your side).
I suggest you, when working on large / long running changes like this one, to check often to be updated!
Sorry again for the inconvenience :-)

This said, I'm making the last checks and then I'll merge it..
Then we can proceed with the other open PRs (quite a load of work from you.. thx!!!)..
Cheers
M.

@dr-prodigy dr-prodigy merged commit 0b0e956 into vacanza:beta Jan 17, 2022
@mborsetti mborsetti deleted the sphinx_docs2 branch January 18, 2022 23:49
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.

3 participants