Skip to content
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 33 commits into from
Mar 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
89d3fe7
Updating github actions
jellis18 Mar 7, 2021
2135633
Update CI
jellis18 Mar 7, 2021
645e350
add numpy cython install
jellis18 Mar 7, 2021
b52dada
Check brew list
jellis18 Mar 7, 2021
264ac0f
Automake
jellis18 Mar 7, 2021
786b6bd
gfortran
jellis18 Mar 7, 2021
01ad509
alias
jellis18 Mar 7, 2021
2d5d411
environment variable
jellis18 Mar 7, 2021
819625e
brew link
jellis18 Mar 7, 2021
1fe00fa
try again
jellis18 Mar 8, 2021
cbc877d
one block
jellis18 Mar 8, 2021
c5a2962
fix typo
jellis18 Mar 8, 2021
c0c639b
indent
jellis18 Mar 8, 2021
0ef8c1e
remove numpy and cython from pyproject
jellis18 Mar 8, 2021
c76dcf1
change jellis-> vallis and add python requirement
jellis18 Mar 13, 2021
76a5f7f
Using logger and adding isort
jellis18 Mar 13, 2021
72fbfe4
isort config
jellis18 Mar 13, 2021
6f8926c
logger.info -> print in docsting
jellis18 Mar 13, 2021
2b36b1e
Adding wheels and pypi
jellis18 Mar 13, 2021
7bf6385
Adding "build requirements" for scikit-sparse
jellis18 Mar 13, 2021
8da5573
Updating github actions and requirements (#3)
jellis18 Mar 13, 2021
d930cbf
Naming to enterprise-pulsar for pypi
jellis18 Mar 14, 2021
e9a2aa5
Merge branch 'master' of https://github.com/jellis18/enterprise
jellis18 Mar 14, 2021
104594f
Small changes to makefile
jellis18 Mar 19, 2021
9e80721
Add coverage percent to test
jellis18 Mar 21, 2021
e0e7672
More cleaning of makefile
jellis18 Mar 21, 2021
171d56c
log-level info in tests
jellis18 Mar 21, 2021
3b3c8c1
Udating contributing
jellis18 Mar 21, 2021
4b61097
Fix contributing.rst
jellis18 Mar 21, 2021
2f0021a
Adding libstempo pypi and dropping CI fail for coverage
jellis18 Mar 24, 2021
f35c2a3
adding no-cover to exceptions
jellis18 Mar 25, 2021
d46f2e5
Updated test
jellis18 Mar 25, 2021
ce403e7
Merge branch 'master' of https://github.com/nanograv/enterprise
jellis18 Mar 25, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 104 additions & 24 deletions .github/workflows/ci_test.yml
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/*
6 changes: 6 additions & 0 deletions .isort.cfg
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
5 changes: 5 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@ repos:
hooks:
- id: flake8
args: ["--config=.flake8"]
- repo: https://github.com/timothycrosley/isort
rev: 5.6.1
hooks:
- id: isort

15 changes: 9 additions & 6 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,17 @@ Ready to contribute? Here's how to set up `enterprise` for local development.

$ git pull upstream master

4. Install your local copy into a virtualenv. Assuming you have virtualenvwrapper installed, this is how you set up your fork for local development::
4. This is how you set up your fork for local development:

.. note::
You will need to have ``tempo`` and ``suitesparse`` installed before
running the commands below.

::

$ mkvirtualenv enterprise
$ cd enterprise/
$ pip install -r requirements_dev.txt
$ pip install -r requirements.txt
$ pip install libstempo --install-option="--with-tempo2=$TEMPO2"
$ python setup.py develop
$ make init
$ source .enterprise/bin/activate

5. Create a branch for local development::

Expand Down
33 changes: 12 additions & 21 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ help:

init:
@python3 -m venv .enterprise --prompt enterprise
@./.enterprise/bin/python3 -m pip install numpy
@./.enterprise/bin/python3 -m pip install -U pip setuptools wheel
@./.enterprise/bin/python3 -m pip install numpy cython
@./.enterprise/bin/python3 -m pip install -r requirements.txt -U
@./.enterprise/bin/python3 -m pip install -r requirements_dev.txt -U
@./.enterprise/bin/python3 -m pip install libstempo --install-option="--with-tempo2=$(TEMPO2)"
@./.enterprise/bin/python3 -m pre_commit install --install-hooks --overwrite
@./.enterprise/bin/python3 -m pip install -e .
@echo "run source .enterprise/bin/activate to activate environment"


format:
black .
Expand Down Expand Up @@ -61,24 +63,20 @@ clean-test: ## remove test and coverage artifacts
rm -fr .tox/
rm -f .coverage
rm -fr htmlcov/
rm -rf coverage.xml

test: ## run tests quickly with the default Python
pytest -v --full-trace --cov-config .coveragerc --cov=enterprise tests

#test-all: ## run tests on every Python version with tox
# tox

coverage: ## check code coverage quickly with the default Python
coverage run --source enterprise setup.py test
COV_COVERAGE_PERCENT ?= 85
test: lint ## run tests quickly with the default Python
pytest -v --durations=10 --full-trace --cov-report html --cov-report xml \
--cov-config .coveragerc --cov-fail-under=$(COV_COVERAGE_PERCENT) \
--cov=enterprise tests

coverage report -m
coverage html
coverage: test ## check code coverage quickly with the default Python
$(BROWSER) htmlcov/index.html

jupyter-docs:
jupyter-docs: ## biuld jupyter notebook docs
jupyter nbconvert --template docs/nb-rst.tpl --to rst docs/_static/notebooks/*.ipynb --output-dir docs/
cp -r docs/_static/notebooks/img docs/
#jupyter nbconvert --template docs/nb-rst.tpl --to rst docs/_static/notebooks/tutorials/*.ipynb --output-dir docs/tutorials/

docs: ## generate Sphinx HTML documentation, including API docs
rm -f docs/enterprise*.rst
Expand All @@ -92,14 +90,7 @@ docs: ## generate Sphinx HTML documentation, including API docs
servedocs: docs ## compile the docs watching for changes
watchmedo shell-command -p '*.rst' -c '$(MAKE) -C docs html' -R -D .

release: clean ## package and upload a release
python setup.py sdist upload
python setup.py bdist_wheel upload

dist: clean ## builds source and wheel package
python setup.py sdist
python setup.py bdist_wheel
ls -l dist

install: clean ## install the package to the active Python's site-packages
python setup.py install
21 changes: 10 additions & 11 deletions enterprise/pulsar.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
import logging
import os

import astropy.units as u
import astropy.constants as const
import astropy.units as u
import numpy as np
from ephem import Ecliptic, Equatorial

Expand All @@ -19,29 +19,28 @@
except:
import pickle

logger = logging.getLogger(__name__)

try:
import libstempo as t2
except ImportError:
print("Ooh, no libstempo?")
logger.warning("libstempo not installed. Will use PINT instead.") # pragma: no cover
Copy link
Contributor Author

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 and PINT as dependencies. Do we want to keep them both or have one as optional?

Copy link
Member

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 within enterprise.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 an ImportError if neither are present?

Copy link
Contributor Author

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:

  1. Don't require either as base dependencies and have a pip install enterprise-pulsar[libstempo] and pip 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.
  2. Choose one as the default and add the other as an extra. Kind of like this one but not sure which should be default
  3. Leave both as it is now. This is fine too

Copy link
Contributor Author

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.

t2 = None

try:
import pint
from pint.toa import TOAs
from pint.models import get_model_and_toas, TimingModel
from pint.models import TimingModel, get_model_and_toas
from pint.residuals import Residuals as resids
from pint.toa import TOAs
except ImportError:
print("Cannot import PINT? Meh...")
logger.warning("PINT not installed. Will use libstempo instead.") # pragma: no cover
pint = None


if pint is None and t2 is None:
err_msg = "Must have either PINT or libstempo timing package installed"
raise ImportError(err_msg)

# logging.basicConfig(format="%(levelname)s: %(name)s: %(message)s", level=logging.INFO)
logger = logging.getLogger(__name__)


def get_maxobs(timfile):
"""Utility function to return number of lines in tim file.
Expand Down Expand Up @@ -119,7 +118,7 @@ def sort_data(self):
"""Sort data by time."""
if self._sort:
self._isort = np.argsort(self._toas, kind="mergesort")
self._iisort = np.zeros(len(self._isort), dtype=np.int)
self._iisort = np.zeros(len(self._isort), dtype=int)
for ii, p in enumerate(self._isort):
self._iisort[p] = ii
else:
Expand Down Expand Up @@ -586,5 +585,5 @@ def Pulsar(*args, **kwargs):
t2pulsar = t2.tempopulsar(relparfile, reltimfile, maxobs=maxobs, ephem=ephem, clk=clk)
os.chdir(cwd)
return Tempo2Pulsar(t2pulsar, sort=sort, drop_t2pulsar=drop_t2pulsar, planets=planets)
else:
print("Unknown arguments {}".format(args))

raise ValueError("Unknown arguments {}".format(args))
1 change: 0 additions & 1 deletion enterprise/signals/selections.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"""Contains various selection functions to mask parameters by backend flags,
time-intervals, etc."""

from __future__ import absolute_import, division, print_function, unicode_literals

import functools
import inspect
Expand Down
14 changes: 7 additions & 7 deletions enterprise/signals/signal_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@
Defines the signal base classes and metaclasses. All signals will then be
derived from these base classes.
"""
from __future__ import absolute_import, division, print_function, unicode_literals

import collections

try:
from collections.abc import Sequence
except:
from collections import Sequence

import itertools
import logging

Expand Down Expand Up @@ -546,10 +545,11 @@ def _setpulsarcliques(self, slices, phis):
try:
self._cliques[slices[sc].start + phiind] = self._clcount
self._clcount = self._clcount + 1
except:
print(self._cliques.shape)
print("phiind", phiind, len(phiind))
print(slices)
except Exception: # pragma: no cover
logger.exception("Exception raised in computing cliques")
logger.info(self._cliques.shape)
logger.info("phiind", phiind, len(phiind))
logger.info(slices)
raise

def get_phi(self, params, cliques=False):
Expand Down Expand Up @@ -684,7 +684,7 @@ def summary(self, include_params=True, to_stdout=False):
summary += "Fixed params: {}\n".format(copcount)
summary += "Number of pulsars: {}\n".format(len(self._signalcollections))
if to_stdout:
print(summary)
logger.info(summary)
else:
return summary

Expand Down
Loading