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.

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

docker: install the same version of pyparsing as on SDCC #264

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

veprbl
Copy link
Member

@veprbl veprbl commented Dec 20, 2021

This is not urgent, but our dockerfiles seem to be broken right now as they install an incompatible version (3.x) of pyparsing needed for mgr/agmlParser.py

pyparsing 3.x had dropped the python 2 support

(cherry picked from commit 816cb54)
@plexoos
Copy link
Member

plexoos commented Dec 23, 2021

How do you see that it is broken?

@veprbl
Copy link
Member Author

veprbl commented Dec 23, 2021

You could try rebuilding the container. Here is my attempt with CentOS 7.3, but it should be the same on SL 7.9:
https://github.com/veprbl/star-sw/runs/4569985951

#16 [base-stage 7/9] RUN pip install pyparsing
#16 0.154 /bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
#16 0.294 sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
#16 0.299 sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
#16 0.374 sh: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
#16 0.387 Collecting pyparsing
#16 0.766   Downloading https://files.pythonhosted.org/packages/ab/61/1a1613e3dcca483a7aa9d446cb4614e6425eb853b90db131c305bd9674cb/pyparsing-3.0.6.tar.gz (882kB)
#16 1.021     Complete output from command python setup.py egg_info:
#16 1.022     Traceback (most recent call last):
#16 1.022       File "<string>", line 1, in <module>
#16 1.022       File "/tmp/pip-build-Mnetrn/pyparsing/setup.py", line 8, in <module>
#16 1.022         from pyparsing import __version__ as pyparsing_version
#16 1.022       File "pyparsing/__init__.py", line 100
#16 1.022         major: int
#16 1.022              ^
#16 1.022     SyntaxError: invalid syntax
#16 1.022     
#16 1.022     ----------------------------------------
#16 1.029 Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-build-Mnetrn/pyparsing/
#16 1.128 You are using pip version 8.1.2, however version 21.3.1 is available.
#16 1.128 You should consider upgrading via the 'pip install --upgrade pip' command.
#16 ERROR: process "/bin/bash -c pip install pyparsing" did not complete successfully: exit code: 1

@klendathu2k
Copy link
Contributor

First, a little history...

The pyparsing package was used to implement the ageParser.py code, which reads in the old AgSTAR geometry files (a.k.a. the ".g" files, kept under pams/geometry). This is the major part of the code which uses pyparsing. If it were the only place, then I think we could safely retire the pyparsing.py code from usage. However, there are minor parts of the syntax (dealing with matching agstar internal variables) which are still present in the AgML parsing framework. So we need to maintain a version of pyparsing in the software stack.

However, I don;t think it matters much which version is used. I expect things haven't changed enough to impact the places where it is needed.

@veprbl
Copy link
Member Author

veprbl commented Jan 19, 2022

However, I don;t think it matters much which version is used. I expect things haven't changed enough to impact the places where it is needed.

Can your code run under python3? If not, then it matters.

@veprbl
Copy link
Member Author

veprbl commented Jan 19, 2022

Also pinning a specific version improves reproducibility.

@klendathu2k
Copy link
Contributor

klendathu2k commented Jan 19, 2022 via email

@plexoos plexoos merged commit 6b1a346 into star-bnl:main Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants