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

MNT: Remove convert_path from setup script #2027

Merged
merged 1 commit into from
Mar 26, 2022

Conversation

greglucas
Copy link
Contributor

This allows building with newer setuptools (61.0.0+) which have removed convert_path, which we don't actually need. Rely on find_packages from setuptools instead.

This allows building with newer setuptools (61.0.0+) which have
removed convert_path, which we don't actually need. Rely on
find_packages instead.
@greglucas greglucas added this to the 0.21 milestone Mar 25, 2022
setup.py Show resolved Hide resolved
Copy link

@glentakahashi glentakahashi left a comment

Choose a reason for hiding this comment

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

Not a maintainer but looks good to me!

Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

Makes sense to me, though I did not check the sdist or wheels; I assume you did?

@greglucas
Copy link
Contributor Author

We only package the sdist, so I think that is all we can compare here. Here is the recursive diff on the two setup.py sdist builds. I looked at the output of the "packages" piece as well and that all looked the same to me at a quick glance, so I think this should be good to go.

diff -r Cartopy-0.20.1.dev110+g22cdafca.d20220326 Cartopy-0.20.1.dev111+g7a9f7f95.d20220326
diff -r Cartopy-0.20.1.dev110+g22cdafca.d20220326/PKG-INFO Cartopy-0.20.1.dev111+g7a9f7f95.d20220326/PKG-INFO
3c3
< Version: 0.20.1.dev110+g22cdafca.d20220326
---
> Version: 0.20.1.dev111+g7a9f7f95.d20220326
diff -r Cartopy-0.20.1.dev110+g22cdafca.d20220326/lib/Cartopy.egg-info/PKG-INFO Cartopy-0.20.1.dev111+g7a9f7f95.d20220326/lib/Cartopy.egg-info/PKG-INFO
3c3
< Version: 0.20.1.dev110+g22cdafca.d20220326
---
> Version: 0.20.1.dev111+g7a9f7f95.d20220326
diff -r Cartopy-0.20.1.dev110+g22cdafca.d20220326/lib/cartopy/_version.py Cartopy-0.20.1.dev111+g7a9f7f95.d20220326/lib/cartopy/_version.py
4,5c4,5
< version = '0.20.1.dev110+g22cdafca.d20220326'
< version_tuple = (0, 20, 1, 'dev110', 'g22cdafca.d20220326')
---
> version = '0.20.1.dev111+g7a9f7f95.d20220326'
> version_tuple = (0, 20, 1, 'dev111', 'g7a9f7f95.d20220326')
diff -r Cartopy-0.20.1.dev110+g22cdafca.d20220326/setup.py Cartopy-0.20.1.dev111+g7a9f7f95.d20220326/setup.py
27d26
< import fnmatch
35c34
< from setuptools import Command, Extension, convert_path, setup
---
> from setuptools import Extension, find_packages, setup
81,105d79
< def find_package_tree(root_path, root_package):
<     """
<     Return the package and all its sub-packages.
< 
<     Automated package discovery - extracted/modified from Distutils Cookbook:
<     https://wiki.python.org/moin/Distutils/Cookbook/AutoPackageDiscovery
< 
<     """
<     packages = [root_package]
<     # Accept a root_path with Linux path separators.
<     root_path = root_path.replace('/', os.path.sep)
<     root_count = len(root_path.split(os.path.sep))
<     for (dir_path, dir_names, _) in os.walk(convert_path(root_path)):
<         # Prune dir_names *in-place* to prevent unwanted directory recursion
<         for dir_name in list(dir_names):
<             if not os.path.isfile(os.path.join(dir_path, dir_name,
<                                                '__init__.py')):
<                 dir_names.remove(dir_name)
<         if dir_names:
<             prefix = dir_path.split(os.path.sep)[root_count:]
<             packages.extend(['.'.join([root_package] + prefix + [dir_name])
<                              for dir_name in dir_names])
<     return packages
< 
< 
347c321
<     packages=find_package_tree('lib/cartopy', 'cartopy'),
---
>     packages=find_packages("lib"),

@greglucas greglucas merged commit 66df268 into SciTools:main Mar 26, 2022
@greglucas greglucas deleted the fix-setup branch March 26, 2022 14:01
@QuLogic
Copy link
Member

QuLogic commented Mar 26, 2022

We only package the sdist

Yes, but pip install will now build wheels and install those, so it's still good to check those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants