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

Add support for Python 3.10 #1049

Merged
merged 5 commits into from
Jan 30, 2022
Merged

Conversation

akrherz
Copy link
Contributor

@akrherz akrherz commented Nov 30, 2021

An attempt to bring Python 3.10 support to maint-1.8 branch? I paired back the original commit some out of my blissful ignorance of what may not be necessary. Please feel free to forcibly reject the PR with fire!

In my fork, the python3.6 build failure matches current maint-1.8 branch build failure

def test_write_memfile_crs_wkt():
E               AssertionError: assert 'SQLite' == 'GPKG'
E                 - SQLite
E                 + GPKG

To get python 3.10 green in CI, I needed to additionally:

  • add a setup.py shim since enum34 was not available.
  • bump pytest and boto3 to a version that works on python 3.10

The please-with-a-cherry-on-top, would be to then cut a 1.8.21 release :)

@mwtoews
Copy link
Member

mwtoews commented Nov 30, 2021

See #1037 for test_write_memfile_crs_wkt; I've just pushed that here to see if that clears the status...

Copy link
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

Looks great, and I'd like to see a release soon too!

@mwtoews mwtoews changed the title Add support for Python 3.10 (#1046) Add support for Python 3.10 Nov 30, 2021
@mwtoews mwtoews added this to the 1.8.21 milestone Nov 30, 2021
@sgillies
Copy link
Member

sgillies commented Dec 8, 2021

Me, too! We don't have wheel-building infrastructure right now and it'll take a few days to set it up. On top of that, I'm not really sure how much it will cost to build the wheels on Travis now since the support for open source projects is gone. $.01? $10? I didn't get any insight into that when I was building the wheels from the Mapbox account.

@@ -295,6 +295,9 @@ def run(self):
'ordereddict; python_version < "2.7"',
'enum34; python_version < "3.4"'
]
# Python 3.10 workaround as enum34 not available
if sys.version_info >= (3, 10):
requirements.remove('enum34; python_version < "3.4"')
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be needed. The intent of enum34; python_version < "3.4" is that enum34 only gets installed for python versions lower than 3.4.

Copy link
Contributor Author

@akrherz akrherz Dec 8, 2021

Choose a reason for hiding this comment

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

I hit a bug without this and how python=3.10 gets compared to python 3.4 -> (python 3.10 < python 3.4) evaluated to true :)

Copy link
Member

Choose a reason for hiding this comment

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

Oof. I dug and found that this bug was fixed 2 years ago: pypa/pip#7166. What version of pip are you using?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't doing anything, it was whatever CI here is using :)

@@ -21,6 +23,7 @@ def test_open_filename_with_exclamation(data_dir):
assert fiona.open(path), "Failed to open !test.geojson"


@requires_gdal21
Copy link
Member

Choose a reason for hiding this comment

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

Tests were passing without this before, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

shrug, maybe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, this change is something @mwtoews added to make things greeen, sorry, I forgot what I did here :)

Copy link
Member

Choose a reason for hiding this comment

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

This is from #1037

Copy link
Member

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

@akrherz Thanks! I have some questions inline.

@rbuffat
Copy link
Contributor

rbuffat commented Dec 11, 2021

Me, too! We don't have wheel-building infrastructure right now and it'll take a few days to set it up. On top of that, I'm not really sure how much it will cost to build the wheels on Travis now since the support for open source projects is gone. $.01? $10? I didn't get any insight into that when I was building the wheels from the Mapbox account.

@sgillies A solution to this would be to build wheels locally. In https://github.com/rbuffat/manylinux_gdal I extended the pypa/manylinux_2_24_x86_64 Docker image with precompiled versions of gdal (inspired from fiona-wheels). With the commands outlined in https://github.com/rbuffat/manylinux_gdal/blob/main/Readme.md it is possible to build wheels locally using Docker. Though, I have not yet tested if they actually work. Probably it will require some more work in order to get it working properly. If you are interested, please contact me so that we can coordinate.

@dopplershift
Copy link

@sgillies It would be really nice to get a fiona that works on 3.10. As far as wheel-building goes, is there anything special you're using Travis for (e.g. powerpc, arm)? Otherwise a lot of the community has moved to Azure and GitHub actions. I'd be happy to lend my expertise on the GitHub Actions side if there's hope of the work getting merged. Currently, this is keeping me from building docs on 3.10 on some of my projects.

@dopplershift dopplershift mentioned this pull request Jan 29, 2022
2 tasks
@sgillies
Copy link
Member

@dopplershift thanks for letting me know! I had assumed that you and your users were getting everything you needed from conda-forge.

Two weeks ago I finished migrating rasterio's wheel infra to GitHub Actions: https://github.com/rasterio/rasterio-wheels. If you've got time, this multibuild configuration could be copied over to a PR against https://github.com/sgillies/fiona-wheels.

@sgillies
Copy link
Member

Apologies for the slowness, everyone! Merging.

@sgillies sgillies merged commit b442840 into Toblerity:maint-1.8 Jan 30, 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

5 participants