search

openzim / python-scraperlib

Collection of Python code to re-use across Python-based scrapers
GNU General Public License v3.0
17 stars 16 forks source link

Contribution instructions in README.md need work #153

Open richterdavid opened 2 months ago

richterdavid commented 2 months ago

https://github.com/openzim/python-scraperlib/blob/7d498319baadba715316c15cf9857ff2f6974a00/README.md?plain=1#L60

  1. My Debian install uses the "externally managed" python3 install option, so it seems that I need pipx rather than pip. Neither was installed and I had to find installation instructions for both. It would be nice to link to them.
  2. pip install ".[dev]" must be run from the local clone of python-scraperlib/.
  3. IIUC pre-commit also requires a pip/pipx install, after which the current line is redundant
  4. invoke is unknown after all that.
$ invoke coverage
-bash: invoke: command not found
benoit74 commented 2 months ago

Thank you. I realize we also do not mention that a local venv is recommended.

@rgaudin do you remember if there is a reason why we did not mentioned it? I would even recommend to use hatch shell to create the venv and install the Python dependencies at once, like we have on ted scraper for instance: https://github.com/openzim/ted?tab=readme-ov-file#getting-started-rocket

My Debian install uses the "externally managed" python3 install option, so it seems that I need pipx rather than pip. Neither was installed and I had to find installation instructions for both. It would be nice to link to them.

We won't be able to delve into the specifics of all OS distributions, but I agree we need to mention that you must have a working python + pip version (and maybe hatch)

pip install ".[dev]" must be run from the local clone of python-scraperlib/.

Of course, all instructions must be ran from the local clone ; I will mention it in the README to make it more obvious

IIUC pre-commit also requires a pip/pipx install, after which the current line is redundant

Nope, pre-commit is installed (i.e. "downloaded" to your machine) by pip install ".[dev]" but the Git hook still has to be configured with pre-commit install

invoke is unknown after all that.

Are you running it from the local clone after pip install ".[dev]" ?

rgaudin commented 2 months ago

@rgaudin do you remember if there is a reason why we did not mentioned it?

No. It all makes perfect sense to me… but I'm not the target audience for this block 😉

Given we're using the bootstrap, I think this should be in the template README (maybe start by copying ted?). We could have a single doc and reference it everywhere but I think directly-readable instructions on the README are a lot better than a link

richterdavid commented 2 months ago

Given my externally managed python3 install, I used pipx to install hatch and .[dev]. With the result that pre-commit was not installed in any visible way. I could certainly start over with venv if that's recommended.

richterdavid commented 2 months ago

Are you running it from the local clone after pip install ".[dev]" ?

yes

~/github/python-scraperlib$ invoke coverage
-bash: invoke: command not found
benoit74 commented 2 months ago

Given my externally managed python3 install, I used pipx to install hatch and .[dev]. With the result that pre-commit was not installed in any visible way. I could certainly start over with venv if that's recommended.

Up to you, you just need to have stuff installed available, otherwise it will obviously fail.