Migrate to python bootstrap conventions

[benoit74]

Rationale

Fix #118
Fix #120

Changes

• migrate to bootstrap conventions (hatch, CI jobs, ruff, ...)
  • including usage of hatch-openzim
  • project metadatas have been slightly changed (author, classifiers) to align with standard
    • some more classifiers should be added in the near future once hatch-openzim permits it (Metadata hook: add support for additional classifiers hatch-openzim#10)
  • tests are ran with --runslow --runinstalled just like before
• added a minimal typing stub for python-libzim:
  • there is mostly nothing for now, just since I had a look and understood how to do it and it is possible to indicate properly in the code that stub is incomplete, I include this in this PR
  • at some point, this will have to be complete so that we do not have anymore typing errors (or pyright ignore rules indeed) due to these missing stubs ;
  • at some point, we will be able to move this to python-libzim (but it is way simpler to do it here for now, instead of always waiting for of python-libzim release and have many back-and-forth)
• all automatic ruff and black fixes have been grouped in one commit
• some ruff fixes have been considered unsafe by the tool so they have been moved to a dedicated commit (only only unsafe rule has been ran, other were not relevant for Python 3.8 support)
• two ruff rules have been completely ignored: "UP006" (PEP585, Python 3.9), "UP007" (PEP604, Python 3.10); they can't be fixed until we drop support for 3.8 and then 3.9
• all remaining Quality issues have been ignored:
  • we will have to fix this on the long term, but doing this in one go is both a very daunting and risky task for coder and for reviewer
  • one commit for "qa" (ruff) issues
  • one commit for "typing" (pyright) issues
• support for Python 3.7 has been dropped and Python 3.12 has been added
• dependencies have been pinned
• "iso-639" library has been replaced by "iso639-lang":
  • "iso-639" was not working with Python 3.12 (use pkg_resources)
  • "iso-639" is not maintained anymore and complex to upgrade due to Python 2 support, with no plan to remove it immediately (see Library is not compatible with Python 3.12 noumar/iso639#16)
  • I had to fix one test because it looks like afa lang has no part2t value in ISO (see https://www.loc.gov/standards/iso639-2/ascii_8bits.html) ; it is not clear if it is a recent change or an original mistake in "iso-639" datasets
  • some glue logic has been necessary to not make a useless breaking change
• some "no cover" pragma where wrongly spelled (and one was misplaced due to code refactoring)

Important notes

• you should probably review commit by commit to gain an understanding of what is going on
• if we agree on the approach on quality issues, I will open a dedicated issue listing all files than have to be fixed to remove all #noqa and #pyright: ignore statements (with a task list, one file per task even if we might sometimes fix multiple files at once)
• the fact that code coverage has been reduced must not be wrongly interpreted ; I did not looked into all details but reduction is mostly due to the fact that code has way more lines now, due to the addition of QA statements and enforcement of the 88 chars line limit

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4dc3012) 100.00% compared to head (ad4b241) 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##              main      #128    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           32        32            
  Lines         1319      1332    +13     
  Branches         0       225   +225     
==========================================
+ Hits          1319      1332    +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rgaudin]

at some point, we will be able to move this to python-libzim

FYI this was not done before because of unsupported types syntax in Cython file. I know things has improved on their side lately so this must be re-assessed. But you're right to add it here for now

[rgaudin]

dependencies have been pinned

That's a mistake. scraperlib is a library and this renders using it very inflexible. See https://github.com/openzim/_python-bootstrap/wiki/pyproject.toml#dependencies

[benoit74]

dependencies have been pinned

That's a mistake. scraperlib is a library and this renders using it very inflexible. See Wiki: pyproject.toml (dependencies) (openzim/_python-bootstrap)

🤦🏻 OMG

That been said, it remembers me that I forgot to specify one thing: the required version in setuptools was open-endeed (i.e. >=3.7). Was this intentional? (hatch-openzim does not support it ...)

[rgaudin]

• is there a way to run tests skipping the slow ones?
• RUF012 should be changed to declare the ClassVar. We don't need mutable dict for presets but it's more readable than using unmutable types.
• FBT002 I think it's a really useful rule… but applying it is a breaking change. Can we open a ticket and switch for next major?
• Next for N818 (NotFound exc)
• We could also then rename getLogger that was chosen to mimic logging's but we dont really care about that.
• no need for runinstalled as we're always installed using our bootstrap
• as discussed, we should aim for 100% coverage, as it was

[benoit74]

As discussed live:

RUF012 should be changed to declare the ClassVar. We don't need mutable dict for presets but it's more readable than using unmutable types.

I prefer the MappingProxyType and will use this approach

• FBT002 I think it's a really useful rule… but applying it is a breaking change. Can we open a ticket and switch for next major?
• Next for N818 (NotFound exc)
• We could also then rename getLogger that was chosen to mimic logging's but we dont really care about that.

N802 and N803 might be considered as well, except the ones of _libkiwix and maybe the ones of config_metadata, I will open a ticket

no need for runinstalled as we're always installed using our bootstrap

Removed

as discussed, we should aim for 100% coverage, as it was

I will fix (it is done indeed now)

[benoit74]

I prefer the MappingProxyType and will use this approach

I finally changed my mind and used ClassVar which is semantically more correct I think

Test coverage is now 100%.

I had to disable the test_user_agent test because it looks like there is a name resolution issue on Github Actions: https://github.com/openzim/python-scraperlib/actions/runs/7869193867/job/21467889432?pr=128 ; I've opened an issue to not forget to reactivate it asap: #129

[rgaudin]

Thank you ;
I see you've added a bunch of tests. Was that all mandated by the new conditional behavior of coverage?

[benoit74]

I see you've added a bunch of tests. Was that all mandated by the new conditional behavior of coverage?

Not all of them, in same case one test was sufficient to provide 100% coverage but it made no sense to not write more tests while working on it (e.g. for test_make_zim_file_working, I'm pretty sure it wasn't needed to test all four combinations to have 100%). But the only driver to look at writing more tests was a situation which wasn't covered by a test.

[rgaudin]

👍