Psychtoolbox-3 icon indicating copy to clipboard operation
Psychtoolbox-3 copied to clipboard

Flake8 alerts

Open DimitriPapadopoulos opened this issue 2 years ago • 1 comments

Describe the bug

Is it worth fixing (some of the) Flake 8 alerts? I can help with that.

Some of them may point to actual issues, for example in regex expressions:

./managementtools/PTB-wikify-into-files.py:271:38: W605 invalid escape sequence '\ '
./managementtools/PTB-wikify-into-files.py:271:56: W605 invalid escape sequence '\ '
./managementtools/PTB-wikify-into-files.py:429:22: W605 invalid escape sequence '\('
./managementtools/PTB-wikify-into-files.py:434:22: W605 invalid escape sequence '\('
./managementtools/PTB-wikify-into-files.py:547:78: W605 invalid escape sequence '\g'
./managementtools/PTB-wikify-into-files.py:549:72: W605 invalid escape sequence '\g'
./managementtools/PTB-wikify-into-files.py:549:78: W605 invalid escape sequence '\g'
./managementtools/PTB-wikify-into-files.py:579:30: W605 invalid escape sequence '\.'
./managementtools/PTB-wikify.py:166:38: W605 invalid escape sequence '\ '
./managementtools/PTB-wikify.py:166:56: W605 invalid escape sequence '\ '
./managementtools/PTB-wikify.py:198:23: W605 invalid escape sequence '\w'
./managementtools/PTB-wikify.py:361:31: W605 invalid escape sequence '\w'
./managementtools/PTB-wikify.py:416:30: W605 invalid escape sequence '\.'

I think Python ends up interpreting \ as \\ in all these cases - so it does what you expect it to do. Yet I think the regex expressions should be reviewed and the Flake8 warnings fixed. The best solution is probably to add r in front of the regex expressions so that they are treated as Python raw strings: https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals

To Reproduce

Run flake8 in the root directory of the project.

Expected behavior

Less Flake8 alerts, especially those that denote actual issues.

Desktop (please complete the following information):

  • Version @master

DimitriPapadopoulos avatar Nov 10 '21 07:11 DimitriPapadopoulos

I don't have the time to look into this, and also i didn't write these scripts - i have zero knowledge about regexp parsing in Python. While i gained quite a bit of knowledge about Python internals and the PyEX compiled C-Extensions api when porting PTB mex files to Python, i'm bloody beginner level at best when it comes to regular Python programming/scripting.

So i don't have an opinion on this either way, all i want is not to get my workflow broken atm.

So if you want to contribute this proposed improvement, you have to test it and make sure there aren't any regressions introduced. The workflow below would probably provide that. I run this on Ubuntu 18.04-LTS atm. with Python 3.6.9:

  1. https://github.com/kleinerm/Psychtoolbox-3/blob/master/managementtools/updateptbdocs.sh is what i use to do documentation updates, with Psychtoolbox-3 and the psychtoolbox website wiki checked out side by side. A simple helper script.
  2. Create such a setup locally, run the scripts as they are, to create a docs update.
  3. Commit the changes to your local documentation / wiki git repo.
  4. Make changes to PTB-Wikify-into-files.py as you want.
  5. Rerun the script.
  6. See if git status + git diff show any new changes to the wiki git repo.
  7. If it shows changes, and they are not without any doubt for the better, then obviously you introduced a regression. git reset --hard the wiki repo, fix your script, goto 4.
  8. No changes, or actual improvments -> Probably no regression introduced. You may send a pull request.

Thanks.

kleinerm avatar Nov 10 '21 21:11 kleinerm

No activity by reporter for along time. Closing as wontfix.

kleinerm avatar Aug 11 '22 00:08 kleinerm