plone / bobtemplates.plone

Python Code Templates for Plone Projects with mr.bob
https://pypi.org/project/bobtemplates.plone/
GNU General Public License v2.0
24 stars 31 forks source link

replace backslash with os.path.sep #535

Closed me-kell closed 1 year ago

me-kell commented 1 year ago

Fixes Issue#531

me-kell commented 1 year ago

Substitutes pull request #532 due to problems with the author's registered e-mail. I apologize for any inconvenience this may have caused.

me-kell commented 1 year ago

I could use some help. I don't understand the check failing. It seems to be caused be the sources without this pull request. Can someone confirm this?

me-kell commented 1 year ago

I'm afraid I'm doing something wrong with this pull request. Could someone give feedback?

yurj commented 1 year ago

try to move imports in this way:

# -*- coding: utf-8 -*-
import codecs
import keyword
import os
import string
import subprocess
import sys
from datetime import date
import six
from colorama import Fore
from colorama import Style
from lxml import etree
from mrbob import hooks
from mrbob.bobexceptions import MrBobError
from mrbob.bobexceptions import SkipQuestion
from mrbob.bobexceptions import ValidationError
from mrbob.rendering import jinja2_env
from six.moves import input

import case_conversion as cc
yurj commented 1 year ago

using forks can be a problem if someone what to contribute to the PR, it is better to do a local branch.

me-kell commented 1 year ago

using forks can be a problem if someone what to contribute to the PR, it is better to do a local branch.

Since the only change was to substitute "/" with os.path.sep I'd probably be easier if I close this pull request and I make a local branch as suggested.

Please confirm if this is ok.

yurj commented 1 year ago

yes. Optionally, you can test the change locally with pylint before committing (pip install pylint; pylint base.py) just to check again if the order of imports is ok.

me-kell commented 1 year ago

yes. Optionally, you can test the change locally with pylint before committing (pip install pylint; pylint base.py) just to check again if the order of imports is ok.

I've created a new branch os_path_sep, committed a single change and documented the change. I have not change any import nor any other code.

The checks are not successful.

What I'm doing wrong?

yurj commented 1 year ago

https://github.com/plone/bobtemplates.plone/pull/535#issuecomment-1336948359

  Error: ERROR: /tmp/pytest-of-runner/pytest-3/test_addon_pattern0/collective.testpattern/src/collective/testpattern/tests/test_robot.py Imports are incorrectly sorted and/or formatted.
me-kell commented 1 year ago

When I run pylint it reports (among other things) that some imports should be replaced. But doesn't changes anything.

When I do isort -v bobtemplates/plone/base.py from the bobtemplates.plone folder it reports the following but doesn't change anything.

from-type place_module for colorama returned THIRDPARTY
from-type place_module for colorama returned THIRDPARTY
from-type place_module for datetime returned STDLIB
from-type place_module for lxml returned THIRDPARTY
from-type place_module for mrbob returned THIRDPARTY
from-type place_module for mrbob.bobexceptions returned THIRDPARTY
from-type place_module for mrbob.bobexceptions returned THIRDPARTY
from-type place_module for mrbob.bobexceptions returned THIRDPARTY
from-type place_module for mrbob.rendering returned THIRDPARTY
from-type place_module for six.moves returned THIRDPARTY
else-type place_module for case_conversion returned THIRDPARTY
else-type place_module for codecs returned STDLIB
else-type place_module for keyword returned STDLIB
else-type place_module for os returned STDLIB
else-type place_module for six returned THIRDPARTY
else-type place_module for string returned STDLIB
else-type place_module for subprocess returned STDLIB
else-type place_module for sys returned STDLIB
from-type place_module for ConfigParser returned THIRDPARTY
from-type place_module for configparser returned STDLIB

if I copy the base.py file elsewhere isort reorders the imports.

I'm not familiar neither with tox nor with isort. Is there any documentation of the workflow for plone projects?

me-kell commented 1 year ago

I am very willing to understand the workflow in this project and to work according to it.

But I really wonder why it is my responsibility to reorder imports that I have not touched and that where previously accepted.

I would also accept this task but I need some guidance.

yurj commented 1 year ago

@MrTango any idea?

MrTango commented 1 year ago

I'm off until Wednesday, i can have a look then.

me-kell commented 1 year ago

As I've already mentioned 10 days ago, this problems seems not to be caused by this pull request.

I could use some help. I don't understand the check failing. It seems to be caused be the sources without this pull request. Can someone confirm this?

to reproduce it

$ tox --version && python3 --version
3.21.4 imported from /usr/lib/python3/dist-packages/tox/__init__.py
Python 3.9.2
$ git clone https://github.com/plone/bobtemplates.plone.git
$ cd bobtemplates.plone/

$ git status
On branch main
Your branch is up to date with 'origin/main'.
nothing to commit, working tree clean

$ tox -e py39-lint
py39-lint installed: flake8==6.0.0,flake8-blind-except==0.2.1,flake8-coding==1.3.2,flake8-commas==2.1.0,flake8-debugger==4.1.2,flake8-deprecated==2.0.1,flake8-html==0.4.2,flake8-pep3101==2.0.0,flake8-plone-api==1.5.0,flake8-plone-hasattr==1.0.0,flake8-print==5.0.0,flake8-quotes==3.3.1,flake8-string-format==0.3.0,flake8-todo==0.7,isort==5.10.1,Jinja2==3.1.2,MarkupSafe==2.1.1,mccabe==0.7.0,pycodestyle==2.10.0,pyflakes==3.0.1,Pygments==2.13.0
py39-lint run-test-pre: PYTHONHASHSEED='1265447768'
py39-lint run-test: commands[0] | mkdir -p /home/map/bobtemplates.plone/_build/reports/flake8
py39-lint run-test: commands[1] | isort --check-only /home/map/bobtemplates.plone/bobtemplates /home/map/bobtemplates.plone/package_tests /home/map/bobtemplates.plone/skeleton-tests setup.py
py39-lint run-test: commands[2] | - flake8 --format=html --htmldir=/home/map/bobtemplates.plone/_build/reports/flake8 --doctests /home/map/bobtemplates.plone/bobtemplates /home/map/bobtemplates.plone/package_tests /home/map/bobtemplates.plone/skeleton-tests setup.py
Traceback (most recent call last):
  File "/home/map/bobtemplates.plone/.tox/py39-lint/bin/flake8", line 8, in <module>
    sys.exit(main())
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/main/cli.py", line 23, in main
    app.run(argv)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/main/application.py", line 198, in run
    self._run(argv)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/main/application.py", line 186, in _run
    self.initialize(argv)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/main/application.py", line 165, in initialize
    self.plugins, self.options = parse_args(argv)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/options/parse_args.py", line 51, in parse_args
    option_manager.register_plugins(plugins)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/options/manager.py", line 259, in register_plugins
    add_options(self)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8_quotes/__init__.py", line 109, in add_options
    cls._register_opt(parser, '--quotes', action='store',
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8_quotes/__init__.py", line 99, in _register_opt
    parser.add_option(*args, **kwargs)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/options/manager.py", line 281, in add_option
    self._current_group.add_argument(*option_args, **option_kwargs)
  File "/usr/lib/python3.9/argparse.py", line 1433, in add_argument
    raise ValueError('%r is not callable' % (type_func,))
ValueError: 'choice' is not callable
py39-lint run-test: commands[3] | flake8 /home/map/bobtemplates.plone/bobtemplates /home/map/bobtemplates.plone/package_tests /home/map/bobtemplates.plone/skeleton-tests setup.py --doctests
Traceback (most recent call last):
  File "/home/map/bobtemplates.plone/.tox/py39-lint/bin/flake8", line 8, in <module>
    sys.exit(main())
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/main/cli.py", line 23, in main
    app.run(argv)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/main/application.py", line 198, in run
    self._run(argv)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/main/application.py", line 186, in _run
    self.initialize(argv)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/main/application.py", line 165, in initialize
    self.plugins, self.options = parse_args(argv)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/options/parse_args.py", line 51, in parse_args
    option_manager.register_plugins(plugins)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/options/manager.py", line 259, in register_plugins
    add_options(self)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8_quotes/__init__.py", line 109, in add_options
    cls._register_opt(parser, '--quotes', action='store',
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8_quotes/__init__.py", line 99, in _register_opt
    parser.add_option(*args, **kwargs)
  File "/home/map/bobtemplates.plone/.tox/py39-lint/lib/python3.9/site-packages/flake8/options/manager.py", line 281, in add_option
    self._current_group.add_argument(*option_args, **option_kwargs)
  File "/usr/lib/python3.9/argparse.py", line 1433, in add_argument
    raise ValueError('%r is not callable' % (type_func,))
ValueError: 'choice' is not callable
ERROR: InvocationError for command /home/map/bobtemplates.plone/.tox/py39-lint/bin/flake8 bobtemplates package_tests skeleton-tests setup.py --doctests (exited with code 1)
__________________________________________________________________________________________ summary ___________________________________________________________________________________________
ERROR:   py39-lint: commands failed
yurj commented 1 year ago

I think you're right and ordering the inputs will solve it.

me-kell commented 1 year ago

$ isort --diff --profile plone .
--- /home/map/bobtemplates.plone/docs/conf.py:before    2022-12-05 15:32:15.828423
+++ /home/map/bobtemplates.plone/docs/conf.py:after 2022-12-05 15:32:37.016683
@@ -12,8 +12,9 @@
 # All configuration values have a default; values that are commented out
 # serve to show the default.

+import os
 import sys
-import os
+

 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
Skipped 1 files

$ isort --profile plone .
Fixing /home/map/bobtemplates.plone/docs/conf.py
Skipped 1 files

$ tox -e py39-lint

 # Same error as before
ERROR: InvocationError for command /home/map/bobtemplates.plone/.tox/py39-lint/bin/flake8 bobtemplates package_tests skeleton-tests setup.py --doctests (exited with code 1)
MrTango commented 1 year ago

I tried to solve this issue in #538, it has to do with the versions of tox and changes in tox config parameter. But somehow there is still a 4.x version in the skeleton test and not as expected a 3.x version. I have to stop now, maybe one of you can see why the newer version is pulled in.

MrTango commented 1 year ago

see this line, where i print the versions: https://github.com/plone/bobtemplates.plone/actions/runs/3750909134/jobs/6371235151#step:8:148 4.0.16 is the newest version of tox, but we need 3.28.0.

me-kell commented 1 year ago

This pull request is obsolete. It has been replaced by https://github.com/plone/bobtemplates.plone/pull/537 which has been accepted and merged today.