Conversation
b0a8332 to
c34ee4c
Compare
|
@larrybradley - I'm happy to take a look at this to see if anything can be simplified further once you rebase :) |
c34ee4c to
be02de0
Compare
|
@astrofrog It's rebased. |
be02de0 to
54062ad
Compare
|
Hello @larrybradley! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2020-05-05 17:21:49 UTC |
54062ad to
220dd11
Compare
425825c to
2bb996a
Compare
666e4bb to
f74c113
Compare
.circleci/config.yml
Outdated
| build: | ||
| docker: | ||
| - image: quay.io/pypa/manylinux1_i686 | ||
| - image: astropy/image-tests-py37-mpl311:1.10 |
There was a problem hiding this comment.
Is the removal of 32-bit testing deliberate?
There was a problem hiding this comment.
No, definitely not. I'm just playing with CircleCI config. I had assumed astropy/image-tests-py37-mpl311:1.10 was 32-bit, but included mpl.
I changed the 32-bit tests to include alldeps, but the issue I'm having is mpl cannot be installed in the quay.io/pypa/manylinux1_i686 image because of a missing freetype library:
https://app.circleci.com/pipelines/github/astropy/photutils/14/workflows/f4936108-6c07-4455-8001-e1d9087499fe/jobs/1149
Unless you know how to fix this, then I'm going revert to running the 32-bit tests only with the required dependencies (which does not include mpl).
There was a problem hiding this comment.
Does the rest of this PR look OK? Everything is ready to be merged, except for this CircleCI issue.
There was a problem hiding this comment.
There isn't a mpl wheel for 32-bit linux, thus CircleCI is compiling from source.
f74c113 to
77cc38c
Compare
Codecov Report
@@ Coverage Diff @@
## master #915 +/- ##
=========================================
Coverage ? 49.01%
=========================================
Files ? 61
Lines ? 6172
Branches ? 0
=========================================
Hits ? 3025
Misses ? 3147
Partials ? 0 Continue to review full report at Codecov.
|
d70c805 to
6abdefa
Compare
6abdefa to
0ef22ae
Compare
|
These changes are based on astropy core and the package template, so I don't think they are controversial. I'm going to merge this PR now because it will fix the failing travis-ci and RTD builds. |
This PR is a followup to #914, simplifying the package setup by completely removing
astropy-helpers. 🚀 Worked on during the pyastro hack day with help from @astrofrog.This PR includes a minimal
toxconfiguration for 'build_docs' and 'test' (both work fine).Some notes:
I discovered that
setuptools_scmcannot be used with packages that still rely onastropy-helpersto build extensions. Theastropy-helpersextension builder looks for theversion.pymodule generated by helpers.I put the Cython extension builds in a separate module called
setup_extensions.py. As discussed with @astrofrog, it would great if a small package could handle building extensions.I added dummy
testandbuild_docssetup commands insetup_commands.pythat print a message to the user that those commands no longer work. Again, it would be nice for that boilerplate to go into a helper package.Also discussed with @astrofrog -- having a
pytestplugin to handle the-Poption is also needed!@astrofrog Please take a look and try it out and let me know if I missed anything.
This PR is not ready to be merged.