Migrate to our new Python standard + fix many subtle issues

[benoit74]

Rationale

Fix #46, i.e. migrate to our new Python standard + upgrade Zimscraperlib to 3.1.1

Changes

• base Docker image is python:3.11-bookworm (upgrade to bookworm)
• zimscraperlib is 3.1.1
• use hatch instead of setuptools
• introduce black / ruff / pyright (basic profile) / pre-commit
• fix many simple formatting / language features / import ordering issues
• fix many subtle bugs raised by ruff / pyright
• publish on releases (instead of tags) to Docker (as before) AND to PyPi (new)
• description is now limited to expected lenght and long description is set
• icons and illustrations are squared as expected

[benoit74]

I'm a bit surprised that publishing to PyPi was not in Github workflow. Was it done manually previously ?
Because it is here and up-to-date: https://pypi.org/project/kolibri2zim/

[rgaudin]

Thank you!

• Sorry about all the extra .encode() that you had to add for add_item_for(). We'll release scraperlib.
• Same goes for all the flake8-errmsg ones.
• Question: what's the reasoning behind your Path().as_posix() usage? Given we don't manipulate paths string outside of pathlib, it sounds a bit cumbersome.

[benoit74]

Thank you !

Sorry about all the extra .encode() that you had to add for add_item_for(). We'll release scraperlib.

Do you have an ETA / release number I could watch? Should we wait for this scraperlib release for merge this PR, or go "as-is" and have a clean-up issue ?

Same goes for all the flake8-errmsg ones.

I'm not sure to get this one. Are you speaking about openzim/_python-bootstrap#17? Does it have something to do with scraperlib?

Question: what's the reasoning behind your Path().as_posix() usage? Given we don't manipulate paths string outside of pathlib, it sounds a bit cumbersome.

We need to transform the Path into a str. Instead of doing a simple str(xxx), I found .as_posix() more meaningfull. I could have used .__fspath__. WDYT?

[rgaudin]

Do you have an ETA / release number I could watch? Should we wait for this scraperlib release for merge this PR, or go "as-is" and have a clean-up issue ?

Released! 3.1.1

Same goes for all the flake8-errmsg ones.

I'm not sure to get this one. Are you speaking about openzim/_python-bootstrap#17? Does it have something to do with scraperlib?

Yes, nothing to do with scraperlib.

Question: what's the reasoning behind your Path().as_posix() usage? Given we don't manipulate paths string outside of pathlib, it sounds a bit cumbersome.

We need to transform the Path into a str. Instead of doing a simple str(xxx), I found .as_posix() more meaningfull. I could have used .__fspath__. WDYT?

It is more meaningful! It implies some posix requirement which is not exact. I think str() is fine

[rgaudin]

Thank you ;

• Dockerfile LABEL is incorrect (repo name)
• User-set description should be checked for length and refused if not compliant.
• User should also be able to set LongDescription (checked as well).
• Description should be user-defined first then from channel (stripped if over limit)
• LongDescription should only be set if user-defined or if description is longer than limit. Otherwise, not set at all.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (main@3adfca2). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #47   +/-   ##
=======================================
  Coverage        ?   24.90%           
=======================================
  Files           ?        7           
  Lines           ?      775           
  Branches        ?      135           
=======================================
  Hits            ?      193           
  Misses          ?      570           
  Partials        ?       12           

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

[benoit74]

I've upgraded to bookworm instead of bullseye. I built the Docker image + perform a run locally, every ran fine (on only few nodes of a channel, but still pretty good sign)

[benoit74]

Could you have a look at all unresolved conversations as well, I think we are ok but would like to have your confirmation before marking as resolved.

[rgaudin]

About ready

[benoit74]

I think I finally understood your point regarding increasing the stack and retaining objects. I reverted multiple changes around this, adding few # pyright: ignore or sensible defaults so that the type checker is ok.

[rgaudin]

Great ; you can change the datetime thing and rule and merge 👍