Skip to content
This repository has been archived by the owner on Aug 11, 2023. It is now read-only.

Refactor package template to follow APE 17 #438

Merged
merged 44 commits into from
Jan 31, 2020

Conversation

astrofrog
Copy link
Member

@astrofrog astrofrog commented Jan 17, 2020

👷‍♂️ This refactors the package template to follow the recommendations in APE 17. Specifically, the changes are:

  • Added the APE 17 migration guide to the docs
  • Removed anything to do with astropy-helpers
  • Added a pyproject.toml file
  • Updated setup.py to match what we have in astropy core
  • Added a tox.ini configuration file
  • Updated Travis CI configuration to use tox
  • Make use of setuptools-scm
  • Removed coveragerc in favor of having the coverage config in setup.cfg
  • Updated conftest.py

In addition I have made the following changes:

  • Added the use_compiled_extensions option for cookiecutter, because now there are some changes that are required for compiled extensions even if there is no example code.
  • Removed include_example_cython_code and instead rely on use_compiled_extensions used together with include_example_code
  • Removed the long_description question in favor of using the README.rst as is often done
  • Removed the automodapi templates which should no longer be needed (not sure why we still had them)

📖 To see a preview of the migration guide, see https://external-builds.readthedocs.io/html/astropy-package-template/438/index.html (or click 'Details' in the status checks for the RTD build)

⚠️ If you want to try out the guide, in Step 0, use the following to use the template from this PR:

cookiecutter gh:astrofrog/package-template --checkout infrastructure-refactor -o my_package_tmp

There is still some cleanup I'd like to make to the template unrelated to APE 17, so I'll leave this to another PR. In particular, @Cadair has been doing some work creating a more generic non-astropy-specific template and as part of that has developed a nice way to test the template, but that can wait for another PR.

Preview of the rendered template: https://github.com/astrofrog/package-template/tree/preview-ape17

@astrofrog astrofrog marked this pull request as ready for review January 17, 2020 22:53
@astrofrog

This comment has been minimized.

@astrofrog astrofrog force-pushed the infrastructure-refactor branch from 4531cc1 to 18b0b77 Compare January 17, 2020 23:04
@astrofrog astrofrog force-pushed the infrastructure-refactor branch from 933c84b to 2af909c Compare January 24, 2020 10:39
@astrofrog astrofrog force-pushed the infrastructure-refactor branch from 762679b to 3430879 Compare January 27, 2020 15:00
@astrofrog astrofrog requested a review from eteq January 27, 2020 16:14
@astrofrog
Copy link
Member Author

This is now ready for review!

hooks/post_gen_project.py Outdated Show resolved Hide resolved
@astrofrog
Copy link
Member Author

Ok I've now simplified the migration guide a bit to avoid repeating whole files when the file can just be copied over as-is from the generated template without any modifications.

@astrofrog
Copy link
Member Author

astrofrog commented Jan 31, 2020

@pllim @mhvk @bsipocz - I've pushed a few more changes:

  • Fixed edit_on_github to always use master
  • Fixed the version specification for the current package in conftest.py
  • Made pytest-astropy-header optional
  • Added a note about why conftest.py has to live in the source tree for now
  • Added a note about updating conda-forge recipes

For the issue of tox not picking up the conftest.py file in time, I'd like to defer this to a follow up PR if possible. I agree that the solution of specifying the packages in setup.cfg and somehow having that information be copied into the installed version would be nice, but I'd like to investigate that in a dedicated PR. I've opened an issue to keep track of this: #439 - so can we ignore it for the purposes of the PR and consider it an ongoing bug?

Otherwise I think I've addressed all the comments so far, but let me know if I missed something!

Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

LGTM! But should wait for @mhvk too.

docs/ape17.rst Outdated Show resolved Hide resolved
Co-Authored-By: P. L. Lim <[email protected]>
@mhvk
Copy link
Contributor

mhvk commented Jan 31, 2020

I don't think I'll have time for another detailed look today, so would suggest to just merge this - it was already great as it was and clearly is now even better! And we have follow-up issues raised already.

Super-nice work!

@pllim pllim merged commit 999de51 into astropy:cookiecutter Jan 31, 2020
@pllim
Copy link
Member

pllim commented Jan 31, 2020

Merged. Thank you, all!

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

Successfully merging this pull request may close these issues.

8 participants