From 3daf293395216ebdd77c54c9d5113b01112d9118 Mon Sep 17 00:00:00 2001 From: Leonardo Uieda Date: Fri, 15 Jun 2018 18:34:37 -1000 Subject: [PATCH] Log error messages with a callback instead of file (#188) Use a callback function passed to `GMT_Create_Session` to log error messages instead of redirecting them to a file using `GMT_Handle_Messages`. It's a lot cleaner and the code is more legible. Other functions don't need to do anything to have their errors logged, it's automatic. The logged messaged are also printed to stderr so they will show up in the Jupyter notebook. This is useful when using the verbose mode (`V="d"`) in modules. Switch to the Fatiando a Terra CI scripts and enable OSX testing on Travis. Fatiando provides scripts for handling all of the CI tasks we need: https://github.com/fatiando/continuous-integration Use them instead of rewriting everything. Fixes #164 The Segmentation fault on OSX was happening because of the log file that we used to capture the GMT output and include in the exception. I have no idea why this happens. But removing that fixes the issue so I'm happy not knowing. --- .stickler.yml | 1 + .travis.yml | 62 +++++++++--------- ci/deploy-docs.sh | 55 ---------------- ci/deploy-pypi.sh | 27 -------- ci/install-miniconda.sh | 23 ------- gmt/clib/core.py | 135 +++++++++++----------------------------- gmt/tests/test_clib.py | 85 +++++++------------------ requirements.txt | 2 +- 8 files changed, 91 insertions(+), 299 deletions(-) delete mode 100755 ci/deploy-docs.sh delete mode 100755 ci/deploy-pypi.sh delete mode 100644 ci/install-miniconda.sh diff --git a/.stickler.yml b/.stickler.yml index c093071d08d..f81f8173029 100644 --- a/.stickler.yml +++ b/.stickler.yml @@ -1,5 +1,6 @@ linters: flake8: + python: 3 enable: true ignore: E203, E266, E501, W503, F401, E741 max-line-length: 88 diff --git a/.travis.yml b/.travis.yml index 9087dc5387d..151b7403844 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,8 +22,7 @@ env: - BUILD_DOCS=false - DEPLOY_DOCS=false - DEPLOY_PYPI=false - # Need the dev channel to get development builds of GMT 6 - - CONDA_FLAGS="--yes --quiet -c conda-forge -c conda-forge/label/dev" + - CONDA_REQUIREMENTS="requirements.txt" matrix: # Build under the following configurations @@ -40,25 +39,24 @@ matrix: - BUILD_DOCS=true - DEPLOY_DOCS=true - DEPLOY_PYPI=true - #- os: osx - #env: - #- PYTHON=3.5 - #- os: osx - #env: - #- PYTHON=3.6 - #- COVERAGE=true - #- BUILD_DOCS=true + - os: osx + env: + - PYTHON=3.5 + - BUILD_DOCS=true + - os: osx + env: + - PYTHON=3.6 + - COVERAGE=true + - BUILD_DOCS=true before_install: - # Get Miniconda from Continuum - # Need to source the script to set the PATH variable in this environment - # (not the scripts environment) - - source ci/install-miniconda.sh - - conda update conda $CONDA_FLAGS - - conda create --name testenv python=$PYTHON pip $CONDA_FLAGS - - source activate testenv - # Install dependencies - - conda install --file requirements.txt $CONDA_FLAGS + # Get the Fatiando CI scripts + - git clone https://github.com/fatiando/continuous-integration.git + # Download and install miniconda and setup dependencies + # Need to source the script to set the PATH variable globaly + - source continuous-integration/travis/setup-miniconda.sh + # Install GMT from the dev channel to get development builds of GMT 6 + - conda install --yes --quiet -c conda-forge/label/dev gmt=6.0.0a* - if [ "$COVERAGE" == "true" ]; then pip install codecov codacy-coverage codeclimate-test-reporter; fi @@ -75,11 +73,11 @@ script: - if [ "$CHECK" == "true" ]; then make check; fi - # Run the test suite + # Run the test suite. Make pytest report any captured output on stdout or stderr. - if [ "$COVERAGE" == "true" ]; then - make coverage; + make coverage PYTEST_EXTRA="-r P"; else - make test; + make test PYTEST_EXTRA="-r P"; fi # Build the documentation - if [ "$BUILD_DOCS" == "true" ]; then @@ -98,26 +96,26 @@ after_success: fi deploy: - # Push the built docs to Github pages + # Make a release on PyPI - provider: script - script: ci/deploy-docs.sh + script: continuous-integration/travis/deploy-pypi.sh + on: + tags: true + condition: '$DEPLOY_PYPI == "true"' + # Push the built HTML in doc/_build/html to the gh-pages branch + - provider: script + script: continuous-integration/travis/deploy-gh-pages.sh skip_cleanup: true on: branch: master condition: '$DEPLOY_DOCS == "true"' - # Push docs when building tags as well + # Push HTML when building tags as well - provider: script - script: ci/deploy-docs.sh + script: continuous-integration/travis/deploy-gh-pages.sh skip_cleanup: true on: tags: true condition: '$DEPLOY_DOCS == "true"' - # Make a release on PyPI - - provider: script - script: ci/deploy-pypi.sh - on: - tags: true - condition: '$DEPLOY_PYPI == "true"' notifications: email: false diff --git a/ci/deploy-docs.sh b/ci/deploy-docs.sh deleted file mode 100755 index c2d51c44597..00000000000 --- a/ci/deploy-docs.sh +++ /dev/null @@ -1,55 +0,0 @@ -#!/bin/bash -# -# Push the built HTML pages to the gh-pages branch. - -# To return a failure if any commands inside fail -set -e - -REPO=gmt-python -USER=GenericMappingTools -BRANCH=gh-pages -CLONE_ARGS="--quiet --branch=$BRANCH --single-branch" -REPO_URL=https://${GH_TOKEN}@github.com/${USER}/${REPO}.git -CLONE_DIR=deploy - -echo -e "Preparing to push HTML to branch ${BRANCH} of ${USER}/${REPO}" - -echo -e "Copying generated files." -cp -R doc/_build/html/ $HOME/keep - -# Go to home and setup git -cd $HOME - -git config --global user.email "travis@nothing.com" -git config --global user.name "TravisCI" - -# Clone the project, using the secret token. -# Uses /dev/null to avoid leaking decrypted key. -echo -e "Cloning ${USER}/${REPO}" -git clone ${CLONE_ARGS} ${REPO_URL} $CLONE_DIR 2>&1 >/dev/null - -cd $CLONE_DIR - -# Move the old branch out of the way and create a new one: -echo -e "Create an empty ${BRANCH} branch" -git checkout ${BRANCH} -git branch -m ${BRANCH}-old -git checkout --orphan ${BRANCH} - -# Delete all the files and replace with our good set -echo -e "Remove old files from previous builds" -git rm -rf . -cp -Rf $HOME/keep/. $HOME/$CLONE_DIR - -# add, commit, and push files -echo -e "Add and commit changes" -git add -f . -git commit -m "Push from Travis build $TRAVIS_BUILD_NUMBER" -echo -e "Pushing..." -git push -fq origin ${BRANCH} 2>&1 >/dev/null - -echo -e "Finished uploading generated files." - -# Workaround for https://github.com/travis-ci/travis-ci/issues/6522 -# Turn off exit on failure. -set +e diff --git a/ci/deploy-pypi.sh b/ci/deploy-pypi.sh deleted file mode 100755 index 09bdb27c640..00000000000 --- a/ci/deploy-pypi.sh +++ /dev/null @@ -1,27 +0,0 @@ -#!/bin/bash -# -# Package and upload to PyPI - -# To return a failure if any commands inside fail -set -e - -echo "" -echo "Building source package and wheels for ${TRAVIS_TAG}" -echo "" -# Build source distributions and wheels -python setup.py sdist bdist_wheel - -echo "" -echo "Packages built:" -ls dist - -echo "" -echo "Deploy to PyPI using twine." -echo "" -# Upload to PyPI. Credentials are set using the TWINCE_PASSWORD and -# TWINE_USERNAME env variables. -twine upload --skip-existing dist/* - -# Workaround for https://github.com/travis-ci/travis-ci/issues/6522 -# Turn off exit on failure. -set +e diff --git a/ci/install-miniconda.sh b/ci/install-miniconda.sh deleted file mode 100644 index d1af1b939da..00000000000 --- a/ci/install-miniconda.sh +++ /dev/null @@ -1,23 +0,0 @@ -#!/bin/bash - -# To return a failure if any commands inside fail -set -e - -MINICONDA_URL="http://repo.continuum.io/miniconda" - -if [ "$TRAVIS_OS_NAME" == "osx" ]; then - MINICONDA_FILE=Miniconda3-latest-MacOSX-x86_64.sh - CONDA_PREFIX=/Users/travis/miniconda3 -else - MINICONDA_FILE=Miniconda3-latest-Linux-x86_64.sh - CONDA_PREFIX=/home/travis/miniconda3 -fi -wget $MINICONDA_URL/$MINICONDA_FILE -O miniconda.sh -chmod +x miniconda.sh -./miniconda.sh -b - -export PATH=$CONDA_PREFIX/bin:$PATH - -# Workaround for https://github.com/travis-ci/travis-ci/issues/6522 -# Turn off exit on failure. -set +e diff --git a/gmt/clib/core.py b/gmt/clib/core.py index 209c8dd8e59..7c00cb96894 100644 --- a/gmt/clib/core.py +++ b/gmt/clib/core.py @@ -1,9 +1,8 @@ """ ctypes wrappers for core functions from the C API """ -import os +import sys import ctypes -from tempfile import NamedTemporaryFile from contextlib import contextmanager from packaging.version import Version @@ -254,10 +253,26 @@ def create_session(self, session_name): restype=ctypes.c_void_p, ) - # None is passed in place of the print function pointer. It becomes the - # NULL pointer when passed to C, prompting the C API to use the default - # print function. - print_func = None + # Capture the output printed by GMT into this list. Will use it later to + # generate error messages for the exceptions raised by API calls. + self._log = [] + + @ctypes.CFUNCTYPE(ctypes.c_int, ctypes.c_void_p, ctypes.c_char_p) + def print_func(file_pointer, message): # pylint: disable=unused-argument + """ + Callback function that GMT uses to print log and error messages. + We'll capture the message and print it to stderr so that it will show up on + the Jupyter notebook. + """ + message = message.decode().strip() + self._log.append(message) + print(message, file=sys.stderr) + return 0 + + # Need to store a copy of the function because ctypes doesn't and it will be + # garbage collected otherwise + self._print_callback = print_func + padding = self.get_constant("GMT_PAD_DEFAULT") session_type = self.get_constant("GMT_SESSION_EXTERNAL") session = c_create_session( @@ -269,6 +284,16 @@ def create_session(self, session_name): return session + def _get_error_message(self): + """ + Return a string with error messages emitted by GMT. + Only includes messages with the string "[ERROR]" in them. + """ + msg = "" + if hasattr(self, "_log"): + msg = "\n".join(line for line in self._log if "[ERROR]" in line) + return msg + def destroy_session(self, session): """ Terminate and free the memory of a registered ``GMTAPI_CTRL`` session. @@ -382,73 +407,6 @@ def get_default(self, name): return value.value.decode() - @contextmanager - def log_to_file(self, logfile=None): - """ - Set the next API call in this session to log to a file. - - Use it as a context manager (in a ``with`` block) to capture the error - messages from GMT API calls. Mostly useful with ``GMT_Call_Module`` - because most others don't print error messages. - - The log file will be deleted when exiting the ``with`` block. - - Calls the GMT API function ``GMT_Handle_Messages`` using - ``GMT_LOG_ONCE`` mode (to only log errors from the next API call). - Only works for a **single API call**, so make calls like - ``get_constant`` outside of the ``with`` block. - - Parameters - ---------- - * logfile : str or None - The name of the logfile. If ``None`` (default), - the file name is automatically generated by the tempfile module. - - Examples - -------- - - >>> with LibGMT() as lib: - ... mode = lib.get_constant('GMT_MODULE_CMD') - ... with lib.log_to_file() as logfile: - ... call_module = lib.get_libgmt_func('GMT_Call_Module') - ... status = call_module(lib.current_session, 'info'.encode(), - ... mode, 'bogus-file.bla'.encode()) - ... with open(logfile) as flog: - ... print(flog.read().strip()) - gmtinfo [ERROR]: Error for input file: No such file (bogus-file.bla) - - """ - c_handle_messages = self.get_libgmt_func( - "GMT_Handle_Messages", - argtypes=[ctypes.c_void_p, ctypes.c_uint, ctypes.c_uint, ctypes.c_char_p], - restype=ctypes.c_int, - ) - - if logfile is None: - tmp_file = NamedTemporaryFile( - prefix="gmt-python-", suffix=".log", delete=False - ) - logfile = tmp_file.name - tmp_file.close() - - status = c_handle_messages( - self.current_session, - self.get_constant("GMT_LOG_ONCE"), - self.get_constant("GMT_IS_FILE"), - logfile.encode(), - ) - if status != 0: - msg = "Failed to set logging to file '{}' (error: {}).".format( - logfile, status - ) - raise GMTCLibError(msg) - - # The above is called when entering a 'with' statement - yield logfile - - # Clean up when exiting the 'with' statement - os.remove(logfile) - def call_module(self, module, args): """ Call a GMT module with the given arguments. @@ -479,30 +437,15 @@ def call_module(self, module, args): ) mode = self.get_constant("GMT_MODULE_CMD") - # If there is no open session, this will raise an exception. Can' let - # it happen inside the 'with' otherwise the logfile won't be deleted. - session = self.current_session - with self.log_to_file() as logfile: - status = c_call_module(session, module.encode(), mode, args.encode()) - # Get the error message inside the with block before the log file - # is deleted - with open(logfile) as flog: - log = flog.read().strip() - # Raise the exception outside the log 'with' to make sure the logfile - # is cleaned. + status = c_call_module( + self.current_session, module.encode(), mode, args.encode() + ) if status != 0: - if log == "": - msg = "Invalid GMT module name '{}'.".format(module) - else: - msg = "\n".join( - [ - "Command '{}' failed:".format(module), - "---------- Error log ----------", - log, - "-------------------------------", - ] + raise GMTCLibError( + "Module '{}' failed with status code {}:\n{}".format( + module, status, self._get_error_message() ) - raise GMTCLibError(msg) + ) def create_data(self, family, geometry, mode, **kwargs): """ @@ -1324,8 +1267,6 @@ def extract_region(self): ) wesn = np.empty(4, dtype=np.float64) - # Use NaNs so that we can know if GMT didn't change the array - wesn[:] = np.nan wesn_pointer = wesn.ctypes.data_as(ctypes.POINTER(ctypes.c_double)) # The second argument to GMT_Extract_Region is a file pointer to a # PostScript file. It's only valid in classic mode. Use None to get a diff --git a/gmt/tests/test_clib.py b/gmt/tests/test_clib.py index 99a29bf5c25..b1094e0fd52 100644 --- a/gmt/tests/test_clib.py +++ b/gmt/tests/test_clib.py @@ -156,51 +156,6 @@ def test_destroy_session_fails(): lib.destroy_session(None) -def test_set_log_file_fails(): - "log_to_file should fail for invalid file names" - with LibGMT() as lib: - with pytest.raises(GMTCLibError): - with lib.log_to_file(logfile=""): - print("This should have failed") - - -def logged_call_module(lib, data_file): - """ - Make a call_module to 'info' with a log file. - The call invalid because 'data_file' doesn't exist. - Checks that the call results in an error and that the correct error message - is logged. - """ - msg = "gmtinfo [ERROR]: Error for input file: No such file ({})" - mode = lib.get_constant("GMT_MODULE_CMD") - with lib.log_to_file() as logfile: - assert os.path.exists(logfile) - # Make a bogus module call that will fail - status = lib._libgmt.GMT_Call_Module( - lib.current_session, "info".encode(), mode, data_file.encode() - ) - assert status != 0 - # Check the file content - with open(logfile) as flog: - log = flog.read() - assert log.strip() == msg.format(data_file) - # Log should be deleted as soon as the with is over - assert not os.path.exists(logfile) - - -def test_errors_sent_to_log_file(): - "Make sure error messages are recorded in the log file." - with LibGMT() as lib: - logged_call_module(lib, "not-a-valid-data-file.bla") - - -def test_set_log_file_twice(): - "Make sure setting a log file twice in a session works" - with LibGMT() as lib: - logged_call_module(lib, "not-a-valid-data-file.bla") - logged_call_module(lib, "another-invalid-data-file.bla") - - def test_call_module(): "Run a command to see if call_module works" data_fname = os.path.join(TEST_DATA_DIR, "points.txt") @@ -213,25 +168,11 @@ def test_call_module(): assert output == "11.5309 61.7074 -2.9289 7.8648 0.1412 0.9338" -def test_call_module_error_message(): - "Check that the exception has the error message from call_module" - data_file = "bogus-data.bla" - true_msg = "\n".join( - [ - "Command 'info' failed:", - "---------- Error log ----------", - "gmtinfo [ERROR]: Error for input file: No such file (bogus-data.bla)", - "-------------------------------", - ] - ) +def test_call_module_invalid_arguments(): + "Fails for invalid module arguments" with LibGMT() as lib: - # Make a bogus module call that will fail - try: - lib.call_module("info", data_file) - except GMTCLibError as error: - assert str(error) == true_msg - else: - assert False, "Didn't raise an exception" + with pytest.raises(GMTCLibError): + lib.call_module("info", "bogus-data.bla") def test_call_module_invalid_name(): @@ -241,6 +182,21 @@ def test_call_module_invalid_name(): lib.call_module("meh", "") +def test_call_module_error_message(): + "Check is the GMT error message was captured." + with LibGMT() as lib: + try: + lib.call_module("info", "bogus-data.bla") + except GMTCLibError as error: + msg = "\n".join( + [ + "Module 'info' failed with status code 71:", + "gmtinfo [ERROR]: Error for input file: No such file (bogus-data.bla)", + ] + ) + assert str(error) == msg + + def test_method_no_session(): "Fails when not in a session" # Create an instance of LibGMT without "with" so no session is created. @@ -593,7 +549,8 @@ def test_virtual_file_fails(): ): with pytest.raises(GMTCLibError): with lib.open_virtual_file(*vfargs): - print("Shouldn't get to this code either") + pass + print("Shouldn't get to this code either") def test_virtual_file_bad_direction(): diff --git a/requirements.txt b/requirements.txt index 12e162c472a..9d34c6aece6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -gmt=6.0.0* +# GMT isn't included because it needs to be downloaded from a separate channel numpy pandas xarray