-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating github actions and requirements #257
Merged
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
89d3fe7
Updating github actions
jellis18 2135633
Update CI
jellis18 645e350
add numpy cython install
jellis18 b52dada
Check brew list
jellis18 264ac0f
Automake
jellis18 786b6bd
gfortran
jellis18 01ad509
alias
jellis18 2d5d411
environment variable
jellis18 819625e
brew link
jellis18 1fe00fa
try again
jellis18 cbc877d
one block
jellis18 c5a2962
fix typo
jellis18 c0c639b
indent
jellis18 0ef8c1e
remove numpy and cython from pyproject
jellis18 c76dcf1
change jellis-> vallis and add python requirement
jellis18 76a5f7f
Using logger and adding isort
jellis18 72fbfe4
isort config
jellis18 6f8926c
logger.info -> print in docsting
jellis18 2b36b1e
Adding wheels and pypi
jellis18 7bf6385
Adding "build requirements" for scikit-sparse
jellis18 8da5573
Updating github actions and requirements (#3)
jellis18 d930cbf
Naming to enterprise-pulsar for pypi
jellis18 e9a2aa5
Merge branch 'master' of https://github.com/jellis18/enterprise
jellis18 104594f
Small changes to makefile
jellis18 9e80721
Add coverage percent to test
jellis18 e0e7672
More cleaning of makefile
jellis18 171d56c
log-level info in tests
jellis18 3b3c8c1
Udating contributing
jellis18 4b61097
Fix contributing.rst
jellis18 2f0021a
Adding libstempo pypi and dropping CI fail for coverage
jellis18 f35c2a3
adding no-cover to exceptions
jellis18 d46f2e5
Updated test
jellis18 ce403e7
Merge branch 'master' of https://github.com/nanograv/enterprise
jellis18 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,52 +1,132 @@ | ||
name: CI-Tests | ||
name: enterprise CI targets | ||
|
||
on: | ||
push: | ||
# branches: | ||
# - master | ||
branches: [ master ] | ||
pull_request: | ||
branches: [ master ] | ||
release: | ||
types: | ||
- published | ||
|
||
|
||
jobs: | ||
lint: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- uses: actions/setup-python@v2 | ||
- uses: psf/black@stable | ||
build: | ||
runs-on: ubuntu-latest | ||
tests: | ||
runs-on: ${{ matrix.os }} | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
os: [ubuntu-latest, macos-latest] | ||
python-version: [3.6, 3.7, 3.8] | ||
|
||
steps: | ||
- name: Checkout Repository | ||
- name: Checkout repository | ||
uses: actions/checkout@v2 | ||
- name: Set up Python ${{ matrix.python-version }} | ||
uses: actions/setup-python@v2 | ||
with: | ||
python-version: ${{ matrix.python-version }} | ||
- name: Pre-install | ||
- name: Install non-python dependencies on mac | ||
if: runner.os == 'macOS' | ||
run: | | ||
brew unlink gcc && brew link gcc | ||
brew install automake suite-sparse | ||
curl -sSL https://raw.githubusercontent.com/vallis/libstempo/master/install_tempo2.sh | sh | ||
- name: Install non-python dependencies on linux | ||
if: runner.os == 'Linux' | ||
run: | | ||
sudo apt-get install libsuitesparse-dev | ||
curl -sSL https://raw.githubusercontent.com/vallis/libstempo/master/install_tempo2.sh | sh | ||
python -m pip install --upgrade pip | ||
- name: Install Dependencies | ||
- name: Install dependencies and package | ||
run: | | ||
cat requirements_dev.txt | xargs -n 1 -L 1 pip install | ||
cat requirements.txt | xargs -n 1 -L 1 pip install | ||
python setup.py install | ||
python -m pip install --upgrade pip setuptools wheel | ||
python -m pip install flake8 pytest black pytest-cov | ||
python -m pip install numpy cython | ||
python -m pip install -e . | ||
- name: Display Python, pip, setuptools, and all installed versions | ||
run: | | ||
python -c "import sys; print(f'Python {sys.version}')" | ||
python -c "import pip; print(f'pip {pip.__version__}')" | ||
python -c "import setuptools; print(f'setuptools {setuptools.__version__}')" | ||
pip freeze | ||
- name: Run Tests and Lint | ||
run: | | ||
pytest --cov-config=.coveragerc --cov=enterprise --cov-report=xml | ||
make lint | ||
python -m pip freeze | ||
- name: Run lint | ||
run: make lint | ||
- name: Test with pytest | ||
run: make test | ||
- name: Codecov | ||
uses: codecov/codecov-action@v1 | ||
#with: | ||
# fail_ci_if_error: true | ||
|
||
|
||
build: | ||
needs: [tests] | ||
name: Build source distribution | ||
runs-on: ubuntu-latest | ||
if: github.event_name == 'release' | ||
|
||
steps: | ||
- uses: actions/checkout@v2 | ||
- name: Set up Python | ||
uses: actions/setup-python@v2 | ||
with: | ||
fail_ci_if_error: true | ||
python-version: "3.7" | ||
- name: Install non-python dependencies on linux | ||
run: | | ||
sudo apt-get install libsuitesparse-dev | ||
curl -sSL https://raw.githubusercontent.com/vallis/libstempo/master/install_tempo2.sh | sh | ||
- name: Build | ||
run: | | ||
python -m pip install --upgrade pip setuptools wheel | ||
pip install numpy==1.16.3 cython | ||
pip install -r requirements.txt | ||
make dist | ||
- name: Test the sdist | ||
run: | | ||
mkdir tmp | ||
cd tmp | ||
python -m venv venv-sdist | ||
venv-sdist/bin/python -m pip install --upgrade pip setuptools wheel | ||
venv-sdist/bin/python -m pip install numpy cython | ||
venv-sdist/bin/python -m pip install ../dist/enterprise*.tar.gz | ||
venv-sdist/bin/python -c "import enterprise;print(enterprise.__version__)" | ||
- name: Test the wheel | ||
run: | | ||
mkdir tmp2 | ||
cd tmp2 | ||
python -m venv venv-wheel | ||
venv-wheel/bin/python -m pip install --upgrade pip setuptools | ||
venv-wheel/bin/python -m pip install numpy cython | ||
venv-wheel/bin/python -m pip install ../dist/enterprise*.whl | ||
venv-wheel/bin/python -c "import enterprise;print(enterprise.__version__)" | ||
- uses: actions/upload-artifact@v2 | ||
with: | ||
name: dist | ||
path: dist/* | ||
|
||
|
||
deploy: | ||
needs: [tests, build] | ||
runs-on: ubuntu-latest | ||
if: github.event_name == 'release' | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- name: Set up Python | ||
uses: actions/setup-python@v2 | ||
with: | ||
python-version: '3.7' | ||
- name: Install dependencies | ||
run: | | ||
python -m pip install --upgrade pip | ||
pip install setuptools wheel twine | ||
- name: Download wheel/dist from build | ||
uses: actions/download-artifact@v2 | ||
with: | ||
name: dist | ||
path: dist | ||
- name: Build and publish | ||
env: | ||
TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }} | ||
TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }} | ||
run: | | ||
twine upload dist/* |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
[settings] | ||
include_trailing_comma=True | ||
indent=' ' | ||
dedup_headings=True | ||
line_length=120 | ||
multi_line_output=3 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nanograv/enterprise These are not really needed if we have both
libstempo
andPINT
as dependencies. Do we want to keep them both or have one as optional?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm glad you changed them to
logger
warnings. I think we could make the case either way. People only need one of the packages to run enterprise. I think the only dependency on either package right now is withinenterprise.Pulsar
, so users can get a way with only using one. I guess that is a pretty rare situation for software. Maybe we just raise anImportError
if neither are present?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I think there are 3 options:
pip install enterprise-pulsar[libstempo]
andpip install enterprise-pulsar[pint]
path. I don't really like this because it could lead to broken installs because the package won't really work without one or the other.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually the whole
Pulsar
function/class is a pretty bad design. I don't think I knew much about class polymorphism at the time. Its too much to refactor it for this PR but it could definitely be better.