Skip to content

Comments

Limit pyparsing to major version 2#471

Merged
pradyunsg merged 2 commits intopypa:mainfrom
moyiz:pyparsing_lt_3
Oct 27, 2021
Merged

Limit pyparsing to major version 2#471
pradyunsg merged 2 commits intopypa:mainfrom
moyiz:pyparsing_lt_3

Conversation

@moyiz
Copy link
Contributor

@moyiz moyiz commented Oct 24, 2021 •
edited
Loading

pyparsing released a new major version (3.0.0) with breaking changes.

Package: https://github.com/pyparsing/pyparsing/
Changelog: https://github.com/pyparsing/pyparsing/blob/master/docs/whats_new_in_3_0_0.rst
Notable changes relate to renaming originalTextFor, stringEnd, stringStart.

This change limits pyparsing to not installl / upgrade to 3.0.0 and resolves the failing test_parseexception_error_msg:

self = <tests.test_requirements.TestRequirements object at 0x7f418a028160>
                                                                               
    def test_parseexception_error_msg(self):                                                                                                                  
        with pytest.raises(InvalidRequirement) as e:                                                                                                          
            Requirement("toto 42")                            
>       assert "Expected stringEnd" in str(e.value)         
E       assert 'Expected stringEnd' in 'Parse error at "\'42\'": Expected string_end'                                                                         
E        +  where 'Parse error at "\'42\'": Expected string_end' = str(InvalidRequirement('Parse error at "\'42\'": Expected string_end'))
E        +    where InvalidRequirement('Parse error at "\'42\'": Expected string_end') = <ExceptionInfo InvalidRequirement('Parse error at "\'42\'": Expected string_end') tblen=2>.value
                                                                                                                                                              
tests/test_requirements.py:195: AssertionError

@pradyunsg
Copy link
Member

pradyunsg commented Oct 24, 2021

(approved the CI run again)

@moyiz
Copy link
Contributor Author

moyiz commented Oct 24, 2021 •
edited
Loading

@pradyunsg Thanks. The tests currently fail because the workflow installs pytest which depends on packaging which sneaks pyparsing==3.0.0 into the test phase. Should the GitHub action be updated to install packaging from "current" branch?

Edit: Meant nox. Added a session.install(".") to use current packaging dependencies. UTs pass in my fork now.

@pradyunsg pradyunsg merged commit 20cd09e into pypa:main Oct 27, 2021
@pradyunsg pradyunsg mentioned this pull request Oct 27, 2021
Closed
@moyiz moyiz deleted the pyparsing_lt_3 branch October 27, 2021 11:53
@henryiii
Copy link
Contributor

henryiii commented Oct 29, 2021

Capping this means that packages that use packaging cannot be installed in the same environment as any library that uses pyparsing>=3.0 - with the renames and fairly large changes, this will probably be quite popular (I was just considering it a day or two ago). This really needs to be fixed to support the newly renamed methods, not capped, not for a library like packaging. Adding a version cap does have consequences.

@henryiii
Copy link
Contributor

henryiii commented Oct 29, 2021 •
edited
Loading

Also, isn't this really just breaking tests that are looking for exact strings in exception messages? Is it breaking any real code? And who's tests are it breaking other than ours?

@layday
Copy link
Member

layday commented Oct 29, 2021

This does only seem to break the one test which is comparing the error message. The release notes say:

Backward-compatibility synonyms for all names and arguments have been included, to allow parsers written using the old names to run without change.

@henryiii
Copy link
Contributor

henryiii commented Oct 29, 2021

Could we just correct this one test to look for both synonyms and then yank 21.2?

@krassowski krassowski mentioned this pull request Oct 29, 2021
Merged
5 tasks
@yardasol yardasol mentioned this pull request Oct 30, 2021
Closed
@moyiz moyiz mentioned this pull request Oct 30, 2021
Closed
This was referenced Nov 1, 2021
Merged
Merged
Merged
Merged
@alemangui alemangui mentioned this pull request Nov 2, 2021
Closed
@moyiz moyiz mentioned this pull request Nov 2, 2021
Merged
@Bento007 Bento007 mentioned this pull request Nov 2, 2021
Merged
This was referenced Nov 4, 2021
Merged
Merged
Merged
Merged
@oliviaruizknott oliviaruizknott mentioned this pull request Nov 5, 2021
Merged
@mlazzarin mlazzarin mentioned this pull request Nov 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@moyiz @pradyunsg @henryiii @layday