Skip to content

Commit

Permalink
Merge #511
Browse files Browse the repository at this point in the history
511: Code refactoring r=MatthieuDartiailh a=MatthieuDartiailh

This is a HUGE pull request. That was not my intention at first but everything ended up this way and there is no clean way to sub divide it. Here is a list of what this PR addresses:

- Code quality changes:

    - use black to format the whole codebase (the initial change is in a single commit allowing to filter it if necessary)
     - use isort to format imports
     - use flake8 to lint the code
    - add type annotations and use mypy to type check the code
    - configure CIs to validate that all the above is done consistently
    - add pre-config to ease the use of those tools for developer
    - use Numpy style doctrsing for all docstrings

- Usability changes:

    - declare VISA attributes on resources rather than magically monkeypatch, should greatly improve the discoverability of those (in particular for serial resources this is important)
    - use more enums to clarify valid inputs for a number of low level functions
    - introduce event classes to wrap event. This provides a nicer interface to access events attributes
    - BREAKING CHANGE: wrap handlers to avoid passing raw ctypes values back (except for user_handle where this cannot be avoided). This is on by default but can be disabled using an environment variable or a global variable.
    - provide a `wrap_handler` method on resources allowing to get directly a resource and event object rather than low-level VISA object.
    - document the way to use events in pyvisa using either the queue mechanism or the handler mechanism.

This is a very large change set but given the number of static analysis tools now running and the fact that when run using the Keysight testing tools the coverage now reaches 93% we should avoid breakages. However I would be happy to see people testing this before I merge. Pinging @hognala, @KOLANICH, @hgrecco 

@jenshnielsen I know that QCoDeS uses mypy and I would be happy to know if this helps or if I got part of the typing wrong.




Co-authored-by: MatthieuDartiailh <marul@laposte.net>
Co-authored-by: mdartiailh <m.dartiailh@gmail.com>
  • Loading branch information
3 people committed Jun 24, 2020
2 parents 55328f8 + 9a63254 commit bac4322
Show file tree
Hide file tree
Showing 74 changed files with 15,054 additions and 7,562 deletions.
92 changes: 92 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
name: Continuous Integration
on:
schedule:
- cron: '0 0 * * 2'
push:
branches:
- master
- staging
- trying
pull_request:
branches:
- master
paths:
- .github/workflows/ci.yml
- pyvisa/*
- pyproject.toml
- setup.cfg
- setup.py

jobs:
formatting:
name: Check code formatting
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Set up Python
uses: actions/setup-python@v1
with:
python-version: 3.8
- name: Install tools
run: |
python -m pip install --upgrade pip
pip install flake8 black isort mypy
- name: Isort
run: |
isort pyvisa -rc -c;
- name: Black
run: |
black pyvisa --check;
- name: Flake8
run: |
flake8 pyvisa;
- name: Mypy
run: |
mypy pyvisa;
tests:
name: Unit tests
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
python-version: [3.6, 3.7, 3.8]
steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v1
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
python -m pip install --upgrade pip
- name: Install project
run: |
pip install -e .
- name: Test with pytest
run: |
pip install pytest-cov
pytest pyvisa/testsuite --cov pyvisa --cov-report xml
- name: Upload coverage to Codecov
uses: codecov/codecov-action@v1
with:
token: ${{ secrets.CODECOV_TOKEN }}
flags: unittests
name: codecov-umbrella
yml: ./codecov.yml
fail_ci_if_error: true

# Added to summarize the matrix (otherwise we would need to list every single
# job in bors.toml)
tests-result:
name: Tests result
if: always()
needs:
- tests
runs-on: ubuntu-latest
steps:
- name: Mark the job as a success
if: needs.tests.result == 'success'
run: exit 0
- name: Mark the job as a failure
if: needs.tests.result != 'success'
run: exit 1
41 changes: 41 additions & 0 deletions .github/workflows/docs.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: Documentation building
on:
schedule:
- cron: '0 0 * * 2'
push:
branches:
- master
- staging
- trying
pull_request:
branches:
- master
paths:
- .github/workflows/docs.yml
- pyvisa/*
- docs/*
- setup.py

jobs:
docs:
name: Docs building
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Set up Python
uses: actions/setup-python@v1
- name: Install dependencies
run: |
python -m pip install --upgrade pip
- name: Install project
run: |
python setup.py develop
- name: Install graphviz
uses: kamiazya/setup-graphviz@v1
- name: Install doc building tools
run: |
pip install sphinx sphinx_rtd_theme
- name: Build documentation
run: |
mkdir docs_output;
sphinx-build docs/source docs_output -W -b html;
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ dist/
MANIFEST
.tox
.coverage
.eggs

# WebDAV file system cache files
.DAV/
_test/
.vscode
htmlcov/
.mypy_cache/
19 changes: 19 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
repos:
- repo: https://github.com/pre-commit/mirrors-isort
rev: v4.3.21
hooks:
- id: isort
- repo: https://github.com/psf/black
rev: stable
hooks:
- id: black
language_version: python3.7
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.3.0
hooks:
- id: flake8
- repo: https://github.com/pre-commit/mirrors-mypy
rev: '' # Use the sha / tag you want to point at
hooks:
- id: mypy
additional_dependencies: [numpy, typing_extensions]
33 changes: 0 additions & 33 deletions .travis.yml

This file was deleted.

20 changes: 19 additions & 1 deletion CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,25 @@ PyVISA Changelog
1.11 (unreleased)
-----------------

PyVISA 1.11 introduces a small backward incompatibility in the handling of
the arguments to handlers for VISA events. The old system was tightly linked to
the ctwrapper and not really usable as it was. As a consequence we pass
the contents of the ctypes objects to the handler rather than the bare ctypes
object (with the exception of the user handle for which there is no way to do
it properly). This is a breaking change and if your code is affected you can revert
to the old behavior by setting the environment variable PYVISA_WRAP_HANDLER=0 or
set ctwrapper.WRAP_HANDLER to False but please consider updating to the new
behavior.

- Add Event class to provide a nice interface to VISA events PR #
- Add `wrap_handler` to provide a nicer way to write event handler PR #
- Add wrapper classes for events PR #
- Add typing to the entire codebase PR #
- Use black and isort on the code to homogenize style PR #
- Convert docstrings to use numpy formatting PR #
- Explicitly set attributes on resources to make teh code much more readable
- Make MessageBasedResource.read_bytes break on message end when
`break_on_termchar` is True PR #
- Add support for dll_extra_paths in .pyvisarc to provide a way to specify paths
in which to look for dll on Windows and Python >= 3.8 PR #509
- Drop Python 2 support PR #486
Expand All @@ -19,7 +38,6 @@ PyVISA Changelog
- Deprecate some unused functions found in the util.py module PR #486
- Warn or raise if the beginning of a binary block is not found among the first characters.
The default value is 25. PR #486
-
- Make the library less biased towards National Instrument by referring to IVI
where relevant. In particular rename the @ni backend to @ivi PR #456
- Deprecate the `visa` module that is causing issue with the VISA payment
Expand Down
20 changes: 8 additions & 12 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ PyVISA
======


.. image:: https://travis-ci.org/pyvisa/pyvisa.svg?branch=master
:target: https://travis-ci.org/pyvisa/pyvisa
:alt: Build Status
.. image:: https://github.com/pyvisa/pyvisa/workflows/Continuous%20Integration/badge.svg
:target: https://github.com/pyvisa/pyvisa/actions
:alt: Continuous integration
.. image:: https://github.com/pyvisa/pyvisa/workflows/Documentation%20building/badge.svg
:target: https://github.com/pyvisa/pyvisa/actions
:alt: Documentation building
.. image:: https://codecov.io/gh/pyvisa/pyvisa/branch/master/graph/badge.svg
:target: https://codecov.io/gh/pyvisa/pyvisa
:alt: Code Coverage
Expand Down Expand Up @@ -70,15 +73,8 @@ controlling:
Requirements
------------

- Python (tested with 2.7 and 3.4+)
- VISA (tested with NI-VISA 17.5, Win7, from www.ni.com/visa)

Python 2 support
----------------

Python 2 EOL is now near (January 1st 2020), and given the limited time
maintainers have, the next release of PyVISA (1.10) to be released around July
2019 will be the last version of PyVISA supporting Python 2.
- Python (tested with 3.6+)
- VISA (tested with NI-VISA 17.5, Win7, from www.ni.com/visa and Keysight-VISA )

Installation
--------------
Expand Down
3 changes: 3 additions & 0 deletions bors.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
status = ["Check code formatting", "Tests result", "Docs building"]
delete-merged-branches = true
timeout_sec = 600
6 changes: 6 additions & 0 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
black
flake8
mypy
isort
sphinx
sphinx-rtd-theme

0 comments on commit bac4322

Please sign in to comment.