move-coop / parsons

A python library of connectors for the progressive community.
Other
254 stars 125 forks source link

Support for python3.12 #1070

Closed jdw25 closed 5 days ago

jdw25 commented 4 weeks ago

This incorporates

austinweisgrau commented 4 weeks ago

Addresses #1069

anzelpwj commented 3 weeks ago
 pytest introspection follows:

Kwargs:
assert {} == {'time_period': 'today'}

IIRC we did discuss that the time_period value did not get passed in to the call. I think we can get rid of the {'time_period': 'today'} is the test.

austinweisgrau commented 3 weeks ago
 pytest introspection follows:

Kwargs:
assert {} == {'time_period': 'today'}

IIRC we did discuss that the time_period value did not get passed in to the call. I think we can get rid of the {'time_period': 'today'} is the test.

See #1072 for a discussion of the underlying issue here. Unfortunately there are several misspecified tests facing the same issue that will need to be rewritten not just to pass but also to test what they are trying to test.

shaunagm commented 2 weeks ago

Minor but important note: you'll also want to update the README.md file to say we support up to 3.12. (I was going to say we need to update the little blue button too but apparently it's dynamically generated from the versions listed on PyPI - not sure how that gets specified but we can figure that out separately)

shaunagm commented 2 weeks ago

Thanks for updating the readme. Also, I looked into it and the blue buttons are getting dynamically generated from the classifiers section of setup.py, which you've already updated in this PR, so we should be all good there.

anzelpwj commented 1 week ago

Add to requirements_dev.txt pycodestyle==2.11.1

austinweisgrau commented 1 week ago

Should be able to remove the test changes from this branch in favor of the more thorough fix in #1072

bmos commented 6 days ago

I wonder why docs build is failing to checkout the PR... https://app.circleci.com/pipelines/github/move-coop/parsons/5443/workflows/01bf4ad9-0f62-48c8-bd31-8720a0505e2d/jobs/7030 Maybe it doesn't like merge commits? It passed for the commit immediately before the merge commit.

jdw25 commented 5 days ago

@bmos yeah, idk maybe!

Based on convo in group pairing meeting yesterday, will retract this PR and create a new one with the cleaner commit sequence (while ending up with same intended changes) and based on the now-merged 1072 (thanks @austinweisgrau!) that should hopefully create a more straightforward path.