Skip to content
This repository has been archived by the owner on Feb 28, 2024. It is now read-only.

Move CI to GitHub Actions #1074

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Move CI to GitHub Actions #1074

wants to merge 20 commits into from

Conversation

kernc
Copy link
Contributor

@kernc kernc commented Oct 4, 2021

Fixes #1060
Closes #1072
Closes #1068
Closes #1069
Fixes #1073

This took a while, but it's often better late than never. 馃槄

The PR looks huge, but it's namely about:

  • migrating the whole of CI onto GitHub Actions (purging TravisCI and CircleCI machinery so we don't have dead code lying around),
  • improving setup.py to aid in releasing (specifying dependencies in one single place, version control by version control etc.),
  • cleaning the code to conform to (hereby finally active!) linting,
  • some documentation consolidation and improvements (README.rst, CONTRIBUTING.md, ISSUE_TEMPLATE.md ...) to help new users.

I tried my best to simplify, yet continue @holgern's work鈥攚hat happened, man, have you died?! :D

It's not too pretty, and it's opinionated in some places, but it's what I can offer and am willing to maintain. The commits have been roughly squashed by function. The PR is at this point certainly ready for review. You can see all the checks passing on a PR to my fork, here.

setup.py: update trove classifiers

setup.py: use setuptools.find_packages()

setup.py: use version from git tag

Note to self: git-tag releases since 0.7.3 !!!

rm MANIFEST.in since handled by setuptools_scm

MNT: setup.py: add dev deps

Remove pyproject.toml and requirements.txt

We already have more versatile setup.py + setup.cfg

Add docs dependencies to setup.py

setup.py: Add project_urls, shown in PyPI sidebar

setup.py: Use SPDX license identifier

Remove doc/requirements.txt in favor of setup.extras_require

setup.py: add pytest-cov dev dependency

CI: Require pytest-xdist for 'test-code-parallel' make target
Add .github/PULL_REQUEST_TEMPLATE.md
Drop CI testing on python 3.6 -- it's EOL by end of the year.

Update README.rst CI badge

DOC: Fix error "Latexmk: Missing input file: 'tgtermes.sty' from line"

CI: Install in develop mode for Docs

Since `make -C doc doctest` is run from the project dir,
sphinx/Python may import the curdir skopt instead of the
installed one.

Install in develop mode to get skopt/_version.py file created.
The alternative would be to cd elsewhere before running make.
Remove media dir; use intro image from doc/image/bo-objective.png

DOC: Merge and extend README/Development into CONTRIBUTING.md

Point docs/development to CONTRIBUTING.md as the canonical source
To v0.22.* -- it was released in 2019.
Also this joblib/joblib#917 (comment)

Futher bump tested scikit-learn == 0.24.* -- it is old enough!

The alternative would be to pin equally ancient scipy to
avoid error:

> AttributeError: 'str' object has no attribute 'decode'
The test was "rediscovered" (in a failing state) by
renaming the test case that follows it.
@pep8speaks
Copy link

pep8speaks commented Oct 4, 2021

Hello @kernc! Thanks for updating this PR. We checked the lines you've touched for PEP聽8 issues, and found:

Line 155:80: E501 line too long (88 > 79 characters)
Line 247:80: E501 line too long (85 > 79 characters)
Line 249:80: E501 line too long (96 > 79 characters)
Line 286:80: E501 line too long (87 > 79 characters)
Line 287:80: E501 line too long (86 > 79 characters)

Line 52:1: E305 expected 2 blank lines after class or function definition, found 1
Line 52:80: E501 line too long (99 > 79 characters)

Line 22:1: E402 module level import not at top of file

Line 113:1: E402 module level import not at top of file

Line 51:1: E402 module level import not at top of file

Line 13:80: E501 line too long (83 > 79 characters)

Line 40:80: E501 line too long (81 > 79 characters)

Comment last updated at 2021-10-11 20:15:31 UTC

Comment on lines 101 to 105
- name: Publish 馃摝 on PyPI
if: env.DO_PUBLISH == 'true'
uses: pypa/gh-action-pypi-publish@master
with:
password: ${{ secrets.PYPI_API_TOKEN }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually, this secret would need to be set to push new packages to PyPI.

Comment on lines 156 to 161
- name: Publish docs
if: env.DO_PUBLISH_DOCS == 'true'
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_DOCS_TOKEN }}
run: |
.github/scripts/publish_docs.sh doc/_build/html/stable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And this secret would need to be set to push generated docs to the docs repo.

@kernc
Copy link
Contributor Author

kernc commented Oct 4, 2021

@pep8speaks in pep8speaks-org/pep8speaks#95 says it should follow setup.cfg flake8 configuration, but apparently not the case?

I'd kindly motion toggling off the noise in favor of a more simple lint check. 馃憤

@@ -0,0 +1,161 @@
name: CI

Choose a reason for hiding this comment

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

As I mentioned in Issue #1060, you're going to want to factorize the CI into separate workflows. GHA was not designed to have everything be shoved into a single config like Travis or Circle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was looking into it, but I found no way to properly DRY/reuse build steps in the publish workflow. They just now made available reusable workflows. Unless you have a better idea for it, I'll look into that. 馃憤

Choose a reason for hiding this comment

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

The reusable workflows are indeed nice, but they are more useful at an org level (though still can be used at the project level if really desired). GHA still are designed to be used in a more modular fashion than Travis or Circle.

@QuentinSoubeyran
Copy link
Contributor

Should scikit-optimize leverage this PR to move to the black code-style, like scikit-learn did ?

@kernc kernc mentioned this pull request Jul 16, 2022
@xmatthias xmatthias mentioned this pull request Nov 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add python_requires to setup.py Migrate CI to GitHub Actions CircleCI forces outdated version of numpy
4 participants