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

Making a 21.3 release #483

Closed
pradyunsg opened this issue Nov 2, 2021 · 20 comments
Closed

Making a 21.3 release #483

pradyunsg opened this issue Nov 2, 2021 · 20 comments

Comments

@pradyunsg
Copy link
Member

I'll pick this up over the weekend, but if someone else with the upload bit on PyPI wants to get to it before me, please do feel free to. :)

Don't forget to update the changelog!

@layday
Copy link
Member

layday commented Nov 8, 2021

FYI pyparsing 3.0.5 breaks requirements with extras, see pyparsing/pyparsing#329, so you might wanna hold off making a new release or exclude 3.0.5.

@ptmcg
Copy link

ptmcg commented Nov 8, 2021

Is there a branch where this pyparsing update work is being done? I'm looking at the master branch, and this code in requirements.py is a likely issue (the internal _original_start and _original_end results names have been renamed).

MARKER_EXPR = originalTextFor(MARKER_EXPR())("marker")
MARKER_EXPR.setParseAction(
    lambda s, l, t: Marker(s[t._original_start : t._original_end])
)

originalTextFor contains its own parse action to do this string slicing, so I'm not sure why this parse action is even being assigned? Same question goes for this code, just a few lines above:

VERSION_SPEC = originalTextFor(_VERSION_SPEC)("specifier")
VERSION_SPEC.setParseAction(lambda s, l, t: t[1])

Though, since this code uses a numeric index instead of the results names, it does not fail. But it also should not be necessary.

Please consider dropping both parse actions, and just using the results from originalTextFor.

@layday
Copy link
Member

layday commented Nov 8, 2021

It appears the intention with MARKER_EXPR.setParseAction is to cast the output to Marker. Would it be better to do it like this:

MARKER_EXPR = originalTextFor(MARKER_EXPR())("marker")
MARKER_EXPR.addParseAction(lambda t: Marker(t.marker))

for it to be called after originalTextFor's default parse action?

No idea what's going on with VERSION_SPEC but the tests don't pass if I take the custom parse action out. It looks like the offsets might be captured in the output.

@ptmcg
Copy link

ptmcg commented Nov 8, 2021

Yes. Using the results name that you have assigned is the preferred way to access the parsed contents.

@layday
Copy link
Member

layday commented Nov 8, 2021

#485 should get requirements with extras working again.

@ptmcg
Copy link

ptmcg commented Nov 8, 2021

Will this fix address the case reported here? pyparsing/pyparsing#329

@layday
Copy link
Member

layday commented Nov 8, 2021

It does, yes.

@brettcannon
Copy link
Member

At the rate that pyparsing is making releases, I wouldn't want to rush out our own release if we are going to get broken again.

Do people know if pyparsing is done making rapid changes and thus #485 is the last thing to merge in?

@ptmcg
Copy link

ptmcg commented Nov 9, 2021

It's a fair question, Brett. The issue has not been so much the frequency of pyparsing releases, it was the fact that releases that should have been minor bugfixes ended up breaking the published API, and so breaking packaging.

The biggest challenge for me in all this was my inability to run packaging tests in my own environment using to-be-released pyparsing code. I have tried following the packaging project's readme for devs, but python -m pytest from the root directory does not run on my system, producing Python tracebacks ending in:

  File "C:\Users\ptmcg\dev\venvs\packaging\lib\site-packages\py\_vendored_packages\apipkg\__init__.py", line 150, in __makeattr
    raise AttributeError(name)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: __spec__

I tried extracting the parser code to test it in isolation, but there are too many tendrils leading back into other packaging .py files.

If I could get help in getting my testing environment for packaging working, I would be able to do further pyparsing work without disrupting your running code.

@layday
Copy link
Member

layday commented Nov 9, 2021

The issue has not been so much the frequency of pyparsing releases, it was the fact that releases that should have been minor bugfixes ended up breaking the published API, and so breaking packaging.

I'm confused, are the two underscored attributes part of the published API? You said they were internal to pyparsing.

producing Python tracebacks ending in

Were you attempting to run the tests on Python 3.11?

@ptmcg
Copy link

ptmcg commented Nov 9, 2021

They aren't part of the published API, as implied by the leading '_' in their names.

I was running Python 3.10 when I got those errors, on my Windows development system.

@ptmcg
Copy link

ptmcg commented Nov 9, 2021

I've just now configured a Ubuntu VM, and I am able to run the full pytest suite there. I will verify all future pyparsing work against this suite in future, to avoid these regressions. I've also opened an issue to do this as part of pyparsing's normal test environment (pyparsing/pyparsing#333).

I am very mindful of the important role that pyparsing plays in packages that use it, and packaging is one of the most prevalent. I hope these measures will improve the stability of pyparsing's place in the Python world.

@layday
Copy link
Member

layday commented Nov 9, 2021

They aren't part of the published API, as implied by the leading '_' in their names.

In that case whatever other changes broke the published API did not impact packaging.

I was running Python 3.10 when I got those errors, on my Windows development system.

You probably have an old version of py installed from pytest which does not support 3.10.

@ptmcg
Copy link

ptmcg commented Nov 9, 2021

Thanks for your patience during this past week's turmoil. With the packaging tests incorporated into pyparsing's regression suite, we shouldn't have any more of these surprise regressions.

@brettcannon
Copy link
Member

Thanks, @ptmcg , for all the info! It sounds like everything should be good, so if we get the various PRs merged we should be able to cut a release when someone has time.

@brettcannon
Copy link
Member

I think if we update the pyparsing requirement to skip 3.0.5 we can do a release as someone is available to (unless I'm missing something?).

@brettcannon brettcannon changed the title Making a 21.2 release Making a 21.3 release Nov 16, 2021
@brettcannon
Copy link
Member

#490 updates the changelog. Assuming CI passes then this should make things in a proper state to do a release.

@brettcannon
Copy link
Member

And thanks to @henryiii for beating me to the PR to skip pyparsing 3.0.5!

@hauntsaninja
Copy link
Contributor

Quick fix for a typo in that CHANGELOG update^ #491

@brettcannon
Copy link
Member

https://pypi.org/project/packaging/21.3/

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

No branches or pull requests

5 participants