Skip to content

Add SWIG to pyproject.toml build dependencies#954

Merged
corranwebster merged 4 commits into
mainfrom
install-swig-from-pypi
Jul 26, 2022
Merged

Add SWIG to pyproject.toml build dependencies#954
corranwebster merged 4 commits into
mainfrom
install-swig-from-pypi

Conversation

@mdickinson
Copy link
Copy Markdown
Member

It looks as though some enterprising soul (Ryan Mast) has helpfully packaged SWIG as a PyPI egg:

This means that if we add SWIG to pyproject.toml, it may be possible to install the next release of Enable from PyPI simply by doing a pip install enable, without having to install SWIG (and worry about SWIG 3 versus SWIG 4) first.

This PR experiments to see whether that works in practice. (I've already experimented locally on my macOS / Intel machine, and it seems to work there.)

run: |
sudo apt-get install swig
python -m pip install -U pip setuptools wheel
python -m pip install numpy
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed numpy and Cython since those too are already mentioned in pyproject.toml, so it shouldn't be necessary to install them separately.

@mdickinson
Copy link
Copy Markdown
Member Author

Hmm. That wasn't much of a test, because we didn't update the EDM-based workflow. I'll do that.

Comment thread ci/edmtool.py
"swig",
"traits",
"traitsui",
"wheel",
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a somewhat independent change: the install of pygarrayimage from PyPI was complaining about having to use a legacy install because wheel wasn't present.

@mdickinson mdickinson changed the title Experiment with installing SWIG from PyPI Add SWIG to pyproject.toml build dependencies Jul 21, 2022
@mdickinson
Copy link
Copy Markdown
Member Author

Title updated: this is no longer experimental, and is ready for review.

@mdickinson
Copy link
Copy Markdown
Member Author

mdickinson commented Jul 21, 2022

I did a manual run of the ETS-from-source workflow on this branch. The build steps worked as expected, so it looks as though the changes in this PR worked. But the test suite failed with a segfault in the same way that it's already been failing for a while.

https://github.com/enthought/enable/actions/runs/2673870586

@mdickinson
Copy link
Copy Markdown
Member Author

mdickinson commented Jul 21, 2022

We could also get rid of the verify_swig_install function in setup.py at this point. OTOH, it's not doing any harm, and could possibly still be useful in the future if there are configuration changes. @jwiggins If this goes in, would you prefer to keep verify_swig_install or to get rid of it?

Copy link
Copy Markdown
Contributor

@corranwebster corranwebster left a comment

Choose a reason for hiding this comment

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

I think this looks good. I successfully pip-installed enable into a clean environment from this branch.

One thing to be perhaps careful about is that cython not being in the dev environment installed by ci/edmtool.py means that other cython features (like HTML annotation of cython code) aren't available unless cython is indepedently installed.

Comment thread pyproject.toml Outdated
@jwiggins
Copy link
Copy Markdown
Member

@jwiggins If this goes in, would you prefer to keep verify_swig_install or to get rid of it?

Can any command line argument be passed to pip such that SWIG would not be present at build time? If the answer is "no" then we should definitely get rid of verify_swig_install

@mdickinson
Copy link
Copy Markdown
Member Author

other cython features (like HTML annotation of cython code) aren't available unless cython is indepedently installed

Good point; I'll add Cython back in.

@mdickinson
Copy link
Copy Markdown
Member Author

Can any command line argument be passed to pip such that SWIG would not be present at build time?

I don't think so. There are other build frontend / backend combinations that could be problematic, but this is all getting a bit hypothetical now. I'll just remove the function.

@corranwebster corranwebster merged commit 999e39c into main Jul 26, 2022
@corranwebster corranwebster deleted the install-swig-from-pypi branch July 26, 2022 15:37
corranwebster pushed a commit that referenced this pull request Jul 26, 2022
* Experiment with installing SWIG from PyPI

* Update the EDM-based workflow

* Remove restriction on SWIG version

* Add Cython back as a development dependency; remove verify_swig_install
corranwebster pushed a commit that referenced this pull request Jul 26, 2022
* Experiment with installing SWIG from PyPI

* Update the EDM-based workflow

* Remove restriction on SWIG version

* Add Cython back as a development dependency; remove verify_swig_install
corranwebster added a commit that referenced this pull request Aug 12, 2022
* Use the "sphinx-copybutton" extension in documentation (#948)

* DOC: Use the sphinx-copybutton extension in documentation

	modified:   ci/edmtool.py
	modified:   docs/source/conf.py
	modified:   enable/__init__.py

* Update ci/edmtool.py

* Get things working on wxPython again (#950)

Addresses these warnigns/exceptions:
- TypeError: BitmapFromImage() got an unexpected keyword argument 'depth'
- AttributeError: 'PaintDC' object has no attribute 'BeginDrawing'
- wxPyDeprecationWarning: Call to deprecated item EmptyImage. Use :class:`Image` instead.
    image = wx.EmptyImage(*sz)
- wxPyDeprecationWarning: Call to deprecated item BitmapFromImage. Use :class:`wx.Bitmap` instead
    bmp = wx.BitmapFromImage(image, depth=-1)

See https://forums.wxwidgets.org/viewtopic.php?t=39930 why the Begin/EndDrawing calls can simply be removed.

Related issues: #798 and #531

* Add SWIG to pyproject.toml build dependencies (#954)

* Experiment with installing SWIG from PyPI

* Update the EDM-based workflow

* Remove restriction on SWIG version

* Add Cython back as a development dependency; remove verify_swig_install

* Remove generated C++ file. (#958)

* Fix Kiva Quartz backend string encoding (#966)

* Add regression test for Enable #964.

* Properly decode strings in Kiva Quartz font code.

* Properly guard against running Quartz tests on non-Mac systems.

* Ensure Wx Quartz test terminates.

* Make test success externally visible.

* Fix tests, use UTF-8.

* Add a github workflow that publishes to sdists to PyPI on release (#967)

* Add a github workflow that publishes to sdists to PyPI on release.

* Use build to build the sdist.

* Take into account all font features when drawing SVG text. (#980)

* Fix Quartz font rendering (#978)

* Use font family in quartz when no font name given.

* Render quartz text with fill color rather than stroke color.

* Fix `SCRIPT` font family in Kiva (#975)

* Add a test for #971

This should fail CI.

* Fix bug and clean-up style.

* Correct first point and orientation of Kiva QPainter arcs (#970)

* Correct first point and orientation of Kiva QPainter arcs.

This fixes #962 but not #960.

* Update kiva/qpainter.py

* Render font families better in QPainter backend (#973)

* Fall back to kiva.fonttools if Qt can't come up with a font name.

* Add a comment about setFamilies()

* Fix WEIGHT_EXTRAHEAVY on Qt6 (#990)

Some changes to tests to make sure eveything is tested correctly.

* Fix the rendering of polygons in basecore2d-based backends (#987)

* Fix the rendering of polygons in basecore2d-based backends

This:

- consistently uses Nx2 numpy arrays to store path points
- doesn't create a new subpath after a LINES function

A driveby fix gives a default implementation for show_text_at_point().

* Clean-up comments.

* Fix some unused imports.

* Some of the unused imports are in fact used.

* Improve the situation of line_state_equal a bit.

* Ensure all subpath arguments are numpy arrays.

* Ensure all backends implement `arc_to` and `quad_curve_to` (#988)

* Ensure all backends implement arc_to and quad_curve_to.

* Fix 'infomration' typo in comments in multiple files.

* Fixes suggested from PR review.

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
Co-authored-by: Brecht Machiels <brecht@mos6581.org>
Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
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.

3 participants