lipoja / URLExtract

URLExtract is python class for collecting (extracting) URLs from given text based on locating TLD.
MIT License
244 stars 61 forks source link

(style): initial commit formatting using black #105

Closed za closed 2 years ago

za commented 3 years ago
➜  URLExtract git:(master) black urlextract

reformatted urlextract/__init__.py
reformatted urlextract/cachefile.py
reformatted urlextract/urlextract_core.py
All done! ✨ 🍰 ✨
3 files reformatted.
za commented 3 years ago

Let me double check the diff again. Please wait @lipoja .

za commented 2 years ago

@lipoja You may double check now. I've manually edited the changes related with \ part.

lipoja commented 2 years ago

@za Hi thanks on formatting the code. However I would like to suggest to add a test to ensure that any future commit is formatted with Black as well. It means to add Black to tox.ini so tests will check if the code is formatted properly or not.

za commented 2 years ago

@lipoja I've updated the PR. Please check.

And BTW, Black requires Python 3.6

Black can be installed by running pip install black. It requires Python 3.6.2+ to run.

https://github.com/psf/black#installation

It failed on Python 3.5

Collecting dnspython
  Downloading dnspython-1.16.0-py2.py3-none-any.whl (188 kB)
ERROR: Could not find a version that satisfies the requirement black
ERROR: No matching distribution found for black
Error: Process completed with exit code 1.

https://github.com/lipoja/URLExtract/runs/4175554130?check_suite_focus=true

lipoja commented 2 years ago

@lipoja I've updated the PR. Please check.

And BTW, Black requires Python 3.6

Black can be installed by running pip install black. It requires Python 3.6.2+ to run.

https://github.com/psf/black#installation

It failed on Python 3.5

Collecting dnspython
  Downloading dnspython-1.16.0-py2.py3-none-any.whl (188 kB)
ERROR: Could not find a version that satisfies the requirement black
ERROR: No matching distribution found for black
Error: Process completed with exit code 1.

https://github.com/lipoja/URLExtract/runs/4175554130?check_suite_focus=true

Please, make Black as a separate test in tox.ini. We do not have to run it in every python environment.

za commented 2 years ago

Please, make Black as a separate test in tox.ini. We do not have to run it in every python environment.

Hi @lipoja can you check? I've made new commit. Sorry for not updating the PR for few days.

I don't have python3.6 running on my machine. So when I run tox, I got this:

black create: /home/za/git/URLExtract/.tox/black
SKIPPED: InterpreterNotFound: python3.6
______________________________________ summary _______________________________________
  py-nocache: commands succeeded
  py-cache: commands succeeded
SKIPPED:  black: InterpreterNotFound: python3.6
  congratulations :)

I have python3.8 running, after I replaced it with python3.8, it's OK:

black create: /home/za/git/URLExtract/.tox/black
black installdeps: black
black inst: /home/za/git/URLExtract/.tox/.tmp/package/1/urlextract-1.4.0.zip
black installed: black==21.11b0,click==8.0.3,filelock==3.4.0,idna==3.3,mypy-extensions==0.4.3,pathspec==0.9.0,platformdirs==2.4.0,regex==2021.11.10,tomli==1.2.2,typing_extensions==4.0.0,uritools==3.0.2,urlextract @ file:///home/za/git/URLExtract/.tox/.tmp/package/1/urlextract-1.4.0.zip
black run-test-pre: PYTHONHASHSEED='1420943709'
black run-test: commands[0] | black urlextract --skip-string-normalization
All done! ✨ 🍰 ✨
3 files left unchanged.
za commented 2 years ago

Hi @lipoja please check. I've addressed the feedback.

lipoja commented 2 years ago

@za Hi, thank you for your time working on this.

za commented 2 years ago

@za Hi, thank you for your time working on this.

No worries, @lipoja ;-)

za commented 2 years ago

Hi @lipoja , I see that Black does not support Python 3.10 yet:

(.env) ➜  URLExtract git:(black-formatted) black --check --skip-string-normalization tests
Usage: black [OPTIONS] SRC ...
Try 'black -h' for help.

Error: Invalid value for '-t' / '--target-version': 'py310' is not one of 'py27', 'py33', 'py34', 'py35', 'py36', 'py37', 'py38', 'py39'.

I'll update pyproject.toml.

za commented 2 years ago

Hi @lipoja I am having issue with this CI process. I might need your help on this.

lipoja commented 2 years ago

Hi @lipoja I am having issue with this CI process. I might need your help on this.

It says that one file is not formatted correctly (would reformat urlextract/urlextract_core.py) Could you run black on the code again and commit the changed file. After that it should pass CI tests.

za commented 2 years ago

Hi @lipoja I am having issue with this CI process. I might need your help on this.

It says that one file is not formatted correctly (would reformat urlextract/urlextract_core.py) Could you run black on the code again and commit the changed file. After that it should pass CI tests.

OK, I added the , again @lipoja . You can check my commit message why I am confused https://github.com/lipoja/URLExtract/pull/105/commits/ea21ee769246f69e9b95c7cc1e446be72e602841

lipoja commented 2 years ago

Hi @lipoja I am having issue with this CI process. I might need your help on this.

It says that one file is not formatted correctly (would reformat urlextract/urlextract_core.py) Could you run black on the code again and commit the changed file. After that it should pass CI tests.

OK, I added the , again @lipoja . You can check my commit message why I am confused ea21ee7

When I remove the comma ',' it is passing tests locally for me. Could you remove the comma, run black again and then push the changes. And lets see what CI tells us. Thanks!

za commented 2 years ago

Hi @lipoja this is the tox result:

black inst-nodeps: /home/za/git/URLExtract/.tox/.tmp/package/1/urlextract-1.4.0.zip
black installed: black==21.11b1,click==8.0.3,filelock==3.4.0,idna==3.3,mypy-extensions==0.4.3,pathspec==0.9.0,platformdirs==2.4.0,regex==2021.11.10,tomli==1.2.2,typing_extensions==4.0.0,uritools==3.0.2,urlextract @ file:///home/za/git/URLExtract/.tox/.tmp/package/1/urlextract-1.4.0.zip
black run-test-pre: PYTHONHASHSEED='435205281'
black run-test: commands[0] | black urlextract --check --skip-string-normalization
would reformat urlextract/urlextract_core.py
Oh no! 💥 💔 💥
1 file would be reformatted, 2 files would be left unchanged.
ERROR: InvocationError for command /home/za/git/URLExtract/.tox/black/bin/black urlextract --check --skip-string-normalization (exited with code 1)
__________________________________________________________________________________ summary __________________________________________________________________________________
  py-nocache: commands succeeded
  py-cache: commands succeeded
ERROR:   black: commands failed

And this is the Black result:

(.env) ➜  URLExtract git:(black-formatted) black urlextract
would reformat urlextract/urlextract_core.py
Oh no! 💥 💔 💥
1 file would be reformatted, 2 files would be left unchanged.
lipoja commented 2 years ago

@za Oh I see one issue ... we can not set "check" for black by default. We have to just use --check only when running black in tox. Otherwise it will not format your files locally.

Could you please remove check = true from pyproject.toml, run black locally and push changes... and could you also include setup.py to formatting and checking (tox). I forgot that before.

za commented 2 years ago

@lipoja can you check again? Now the tox also failed.

za commented 2 years ago

Hi @lipoja once you have time, please check the PR. Need your advise/input :-)

lipoja commented 2 years ago

Thanks for this PR! I will deal with those errors.

lipoja commented 2 years ago

Thanks for this PR! I will deal with those errors.

za commented 2 years ago

Wow, thanks for merging it @lipoja I thought we should've done some work before merging this PR.

Let me know how will you handle and tackle the errors.