Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove several mentions of Python 2 #190

Merged
merged 2 commits into from
Jun 16, 2023
Merged

Remove several mentions of Python 2 #190

merged 2 commits into from
Jun 16, 2023

Conversation

jafingerhut
Copy link
Collaborator

Not all of them are removed with these changes, because I am not sure how best to remove some of them, e.g. in the file debian/control

@jafingerhut
Remove several mentions of Python 2
0ea8d44 
Not all of them are removed with these changes, because I am not sure
how best to remove some of them, e.g. in the file debian/control
@jafingerhut
Copy link
Collaborator Author

This is at least a partial fix for issue #186

I did not remove all mentions of Python2, because I was not sure what the correct way to do so was for some of them, e.g. the one in the file debian/control

antoninbas
antoninbas reviewed Jun 15, 2023
View reviewed changes
Copy link
Member

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the debian/ has not been updated for 7 years, so it's not a concern for me.
However, I think as part of this change, we should also remove the six library dependency. And update

ptf/setup.cfg

Line 22 in 9c1787d

python_requires = >=2.7, >=3

to remove the Python 2.7 mention

@jafingerhut
Copy link
Collaborator Author

Removal of setup.cfg of mention of Python 2.7 was part of first commit already. Commit 2 removes use of six package.

antoninbas
antoninbas approved these changes Jun 16, 2023
View reviewed changes
ptf_nn/ptf_nn_agent.py
@@ -305,10 +305,7 @@ def forward(self, p, port):
msg = struct.pack("<iii{}s".format(len(p)), self.MSG_TYPE_PACKET_OUT,
port, len(p), p)
# because nnpy expects unicode when using str
if sys.version_info[0] == 2:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice job finding this one :P

src/ptf/testutils.py
@@ -3179,7 +3179,7 @@ def ptf_ports(num=None):


def port_to_tuple(port):
if type(port) is int or (sys.version_info[0] == 2 and type(port) is int):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one must have been generated by some 2-to-3 tool...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the git commit history on this one, since it looked so strange. At one point it was less weird looking, I think something like is long in the last part instead of is int.

@antoninbas antoninbas merged commit 6e1a7c5 into p4lang:main Jun 16, 2023
@jafingerhut jafingerhut mentioned this pull request Jun 16, 2023
Closed
@liushilongbuaa
Copy link

liushilongbuaa commented Jun 16, 2023 •
edited
Loading 

# git log -n 1 
commit bb28a88187ca9195884dae319e8a660838de948b (HEAD -> main, origin/main, origin/HEAD)
Author: Antonin Bas <[email protected]>
Date:   Thu Jun 15 18:46:31 2023 -0700

    Only push tagged versions to PyPI (#191)
    
    Signed-off-by: Antonin Bas <[email protected]>
# git status
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean
# python3 setup.py install --single-version-externally-managed --record /tmp/ptf_install.txt
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/packaging/requirements.py", line 90, in __init__
    req = REQUIREMENT.parseString(requirement_string)
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 1654, in parseString
    raise exc
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 1644, in parseString
    loc, tokens = self._parse( instring, 0 )
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 1402, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 3417, in parseImpl
    loc, exprtokens = e._parse( instring, loc, doActions )
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 1406, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 3205, in parseImpl
    raise ParseException(instring, loc, self.errmsg, self)
pkg_resources._vendor.pyparsing.ParseException: Expected stringEnd (at char 4), (line:1, col:5)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 3042, in __init__
    super(Requirement, self).__init__(requirement_string)
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/packaging/requirements.py", line 94, in __init__
    requirement_string[e.loc:e.loc + 8]))
pkg_resources.extern.packaging.requirements.InvalidRequirement: Invalid requirement, parse error at "': requir'"

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "setup.py", line 4, in <module>
    setuptools.setup()
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 144, in setup
    _install_setup_requires(attrs)
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 137, in _install_setup_requires
    dist.parse_config_files(ignore_option_errors=True)
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 706, in parse_config_files
    self._finalize_requires()
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 506, in _finalize_requires
    self._move_install_requirements_markers()
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 545, in _move_install_requirements_markers
    inst_reqs = list(pkg_resources.parse_requirements(spec_inst_reqs))
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 3035, in parse_requirements
    yield Requirement(line)
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 3044, in __init__
    raise RequirementParseError(str(e))
pkg_resources.RequirementParseError: Invalid requirement, parse error at "': requir'"
# git diff
diff --git a/setup.cfg b/setup.cfg
index e1fda97..f8ff662 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -22,7 +22,6 @@ platforms = any
 python_requires = >=3
 setup_requires =
     setuptools_scm
-install_requires = file: requirements.txt
 
 [options.packages.find]
 where = src
# python3.7 setup.py install --single-version-externally-managed --record /tmp/ptf_install.txt
running install
running build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoninbas antoninbas antoninbas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jafingerhut @liushilongbuaa @antoninbas