Skip to content

Move to a single setup.py file#535

Merged
jwiggins merged 4 commits into
masterfrom
infra/unified-setup-py
Jan 29, 2021
Merged

Move to a single setup.py file#535
jwiggins merged 4 commits into
masterfrom
infra/unified-setup-py

Conversation

@jwiggins
Copy link
Copy Markdown
Member

@jwiggins jwiggins commented Jan 8, 2021

Closes #440

This brings all the extension building into the single top-level setup.py script.

Some changes:

  • No more support for 32-bit OSX systems. That ship has sailed.
  • No more ALWAYS_32BIT_WORKAROUND for 64-bit machines with g++ < 4.0
  • The macport extension, used for Wx 2.6 support, is not longer built.
  • The --compiler=STR option for setup.py develop has been removed.
  • The custom clean and build_py commands are gone, as is the explicit use of numpy distutils.

NOTE: The sdist looks like it might be broken.
NOTE: This conflicts pretty massively with #392

@kitchoi
Copy link
Copy Markdown
Contributor

kitchoi commented Jan 8, 2021

Offline discussion: We may want to consider this as a proof-of-concept given the nontrivial conflicts with #392.

@kitchoi
Copy link
Copy Markdown
Contributor

kitchoi commented Jan 8, 2021

We may want to consider this as a proof-of-concept given the nontrivial conflicts with #392.

Sorry that needs a bit more clarification. This PR is considered provisional now. We need a resolution about #392 first.

@jwiggins jwiggins force-pushed the infra/unified-setup-py branch 7 times, most recently from b606f82 to d43a1d2 Compare January 8, 2021 13:56
@jwiggins
Copy link
Copy Markdown
Member Author

jwiggins commented Jan 8, 2021

Finally. The installation is failing because of the .py files generated by SWIG are not copied:

[...]
adding 'kiva/agg/__init__.py'
adding 'kiva/agg/_agg.cpython-36m-x86_64-linux-gnu.so'
adding 'kiva/agg/_plat_support.cpython-36m-x86_64-linux-gnu.so'
adding 'kiva/agg/tests/__init__.py'
[...]

(You should see kiva/agg/agg.py and kiva/agg/plat_support.py in the list of files)

Just need to figure out how to get SWIG-generated .py files into the distribution and this should be working.

(The exact same problem as described here: https://stackoverflow.com/questions/64638474/pythons-setuptools-doesnt-include-auto-generated-swig-python-files)

@jwiggins
Copy link
Copy Markdown
Member Author

jwiggins commented Jan 8, 2021

Found an answer: https://stackoverflow.com/a/21236111

@jwiggins jwiggins force-pushed the infra/unified-setup-py branch 2 times, most recently from 0cc303e to 1e9c515 Compare January 8, 2021 17:36
@rahulporuri rahulporuri self-requested a review January 27, 2021 07:42
@jwiggins jwiggins force-pushed the infra/unified-setup-py branch from ac1f664 to fef6d32 Compare January 27, 2021 10:18
@rahulporuri rahulporuri removed their request for review January 28, 2021 09:16
@jwiggins jwiggins force-pushed the infra/unified-setup-py branch from fef6d32 to d4bc4ec Compare January 28, 2021 10:34
@jwiggins jwiggins force-pushed the infra/unified-setup-py branch from d4bc4ec to cce8e3b Compare January 28, 2021 11:10
@jwiggins
Copy link
Copy Markdown
Member Author

This PR has now integrated the changes from #392 and is ready for review!

@jwiggins jwiggins requested a review from rahulporuri January 28, 2021 11:24
Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM. I tested this locally (windows, python 3.6 env) - with and without wheel in the environment - and the instllation went without a hitch.

I could mostly follow the changes between the deletions and the additions - but I trust the CI.

Thanks again @jwiggins for this crucial work.

@jwiggins
Copy link
Copy Markdown
Member Author

Out of an abundance of caution, I diffed the SDISTs between this branch and master and found a couple changes that needed to be made.

@jwiggins
Copy link
Copy Markdown
Member Author

Thanks for the review!

@jwiggins jwiggins merged commit 92e12a4 into master Jan 29, 2021
@jwiggins jwiggins deleted the infra/unified-setup-py branch January 29, 2021 09:54
@rahulporuri
Copy link
Copy Markdown
Contributor

Out of an abundance of caution, I diffed the SDISTs between this branch and master and found a couple changes that needed to be made.

Thank you. Not sure why that didn't occur to me :/

@jwiggins
Copy link
Copy Markdown
Member Author

Not sure why that didn't occur to me

Not enough painful memories to draw from?

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.

Clean up Enable/Kiva setup.py

3 participants