Skip to content

MRG: refactoring for pydicom 1.0 #599

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 19, 2018
Merged

MRG: refactoring for pydicom 1.0 #599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d83d81c
RF: refactoring to account for pydicom 1 changes.
matthew-brett Feb 17, 2018
7ff109c
MAINT: Refactor pydicom CI
matthew-brett Feb 18, 2018
618d02a
MAINT: Update minimum pydicom version
matthew-brett Feb 18, 2018
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
27 changes: 9 additions & 18 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ cache:
- $HOME/.cache/pip
env:
global:
- DEPENDS="six numpy scipy matplotlib h5py pillow"
- DEPENDS="six numpy scipy matplotlib h5py pillow pydicom"
- OPTIONAL_DEPENDS=""
- PYDICOM=1
- INSTALL_TYPE="setup"
- EXTRA_WHEELS="https://5cf40426d9f06eb7461d-6fe47d9331aba7cd62fc36c7196769e4.ssl.cf2.rackcdn.com"
- PRE_WHEELS="https://7933911d6844c6c53a7d-47bd50c35cd79bd838daf386af554a83.ssl.cf2.rackcdn.com"
Expand All @@ -34,29 +33,29 @@ matrix:
# Absolute minimum dependencies
- python: 2.7
env:
- DEPENDS="numpy==1.7.1" PYDICOM=0
- DEPENDS="numpy==1.7.1"
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't the idea here to make sure that we didn't break when pydicom was absent? Do we want to move to a pydicom dependency?

Copy link
Member

Choose a reason for hiding this comment

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

Wow, brain failure. I see this replaces the pydicom dependency above. Carry on.

Copy link
Member Author

Choose a reason for hiding this comment

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

With this matrix entry, Pydicom isn't in the DEPENDS variable, and doesn't get installed. On the way, the tests did in fact fail when Pydicom wasn't present, I had to fix the code to make it work again. Have a look at the Travis-CI log to confirm this entry does not install Pydicom...

# Absolute minimum dependencies plus oldest MPL
# Check these against:
# nibabel/info.py
# doc/source/installation.rst
# requirements.txt
- python: 2.7
env:
- DEPENDS="numpy==1.7.1 matplotlib==1.3.1" PYDICOM=0
- DEPENDS="numpy==1.7.1 matplotlib==1.3.1"
# Minimum pydicom dependency
- python: 2.7
env:
- DEPENDS="numpy==1.7.1 pydicom==0.9.7 pillow==2.6"
- DEPENDS="numpy==1.7.1 pydicom==0.9.9 pillow==2.6"
# test against numpy 1.7
- python: 2.7
env:
- DEPENDS="numpy==1.7.1"
# pydicom 1.0 (currently unreleased)
- python: 2.7
# pydicom master branch
- python: 3.5
env:
- PYDICOM="v1.0"
# test 2.7 against pre-release builds of everything
- python: 2.7
- DEPENDS="numpy git+https://github.com/pydicom/pydicom.git@master"
# test 3.5 against pre-release builds of everything
- python: 3.5
env:
- EXTRA_PIP_FLAGS="$PRE_PIP_FLAGS"
# test 3.5 against pre-release builds of everything
Expand Down Expand Up @@ -102,14 +101,6 @@ before_install:
- pip install -U pip wheel # needed at one point
- retry pip install nose flake8 mock # always
- pip install $EXTRA_PIP_FLAGS $DEPENDS $OPTIONAL_DEPENDS
# pydicom <= 0.9.8 doesn't install on python 3
- if [ "${TRAVIS_PYTHON_VERSION:0:1}" == "2" ]; then
if [ "$PYDICOM" == "1" ]; then
pip install pydicom;
elif [ "$PYDICOM" == "v1.0" ]; then
pip install git+https://github.com/darcymason/pydicom.git@43f278444d5cb2e4648135d3edcd430c363c6975;
fi
fi
- if [ "${COVERAGE}" == "1" ]; then
pip install coverage;
pip install coveralls;
Expand Down
2 changes: 1 addition & 1 deletion doc/source/installation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ Requirements
* NumPy_ 1.7.1 or greater
* Six_ 1.3 or greater
* SciPy_ (optional, for full SPM-ANALYZE support)
* PyDICOM_ 0.9.7 or greater (optional, for DICOM support)
* PyDICOM_ 0.9.9 or greater (optional, for DICOM support)
* `Python Imaging Library`_ (optional, for PNG conversion in DICOMFS)
* nose_ 0.11 or greater (optional, to run the tests)
* mock_ (optional, to run the tests)
Expand Down
2 changes: 1 addition & 1 deletion nibabel/info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def cmp_pkg_version(version_str, pkg_version_str=__version__):
# requirements.txt
# .travis.yml
NUMPY_MIN_VERSION = '1.7.1'
PYDICOM_MIN_VERSION = '0.9.7'
PYDICOM_MIN_VERSION = '0.9.9'
SIX_MIN_VERSION = '1.3'

# Main setup parameters
Expand Down
4 changes: 2 additions & 2 deletions nibabel/nicom/dicomwrappers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from .dwiparams import B2q, nearest_pos_semi_def, q2bg
from ..openers import ImageOpener
from ..onetime import setattr_on_read as one_time
from ..pydicom_compat import pydicom
from ..pydicom_compat import tag_for_keyword


class WrapperError(Exception):
Expand Down Expand Up @@ -517,7 +517,7 @@ def image_shape(self):
# Determine if one of the dimension indices refers to the stack id
dim_seq = [dim.DimensionIndexPointer
for dim in self.get('DimensionIndexSequence')]
stackid_tag = pydicom.datadict.tag_for_name('StackID')
stackid_tag = tag_for_keyword('StackID')
# remove the stack id axis if present
if stackid_tag in dim_seq:
stackid_dim_idx = dim_seq.index(stackid_tag)
Expand Down
7 changes: 4 additions & 3 deletions nibabel/nicom/tests/test_dicomwrappers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

import numpy as np

from nibabel.pydicom_compat import have_dicom, pydicom, read_file, dicom_test
from nibabel.pydicom_compat import (have_dicom, pydicom, read_file, dicom_test,
tag_for_keyword)

from .. import dicomwrappers as didw
from .. import dicomreaders as didr
Expand Down Expand Up @@ -416,8 +417,8 @@ def __init__(self, div, sid):
dim_idx_seq = [DimIdxSeqElem()] * num_of_frames
# add an entry for StackID into the DimensionIndexSequence
if sid_dim is not None:
sid_tag = pydicom.datadict.tag_for_name('StackID')
fcs_tag = pydicom.datadict.tag_for_name('FrameContentSequence')
sid_tag = tag_for_keyword('StackID')
fcs_tag = tag_for_keyword('FrameContentSequence')
dim_idx_seq[sid_dim] = DimIdxSeqElem(sid_tag, fcs_tag)
# create the PerFrameFunctionalGroupsSequence
frames = [PerFrmFuncGrpSeqElem(div, sid)
Expand Down
15 changes: 12 additions & 3 deletions nibabel/pydicom_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,18 @@
* pydicom : pydicom module or dicom module or None of not importable;
* read_file : ``read_file`` function if pydicom or dicom module is importable
else None;
* tag_for_keyword : ``tag_for_keyword`` function if pydicom or dicom module
is importable else None;
* dicom_test : test decorator that skips test if dicom not available.
"""

# Module does (apparently) unused imports; stop flake8 complaining
# Module has (apparently) unused imports; stop flake8 complaining
# flake8: noqa

import numpy as np

have_dicom = True
read_file = None
pydicom = None
pydicom = read_file = tag_for_keyword = None

try:
import dicom as pydicom
Expand All @@ -38,6 +39,14 @@
else: # dicom module available
read_file = pydicom.read_file

if have_dicom:
try:
# Versions >= 1.0
tag_for_keyword = pydicom.datadict.tag_for_keyword
except AttributeError:
# Versions < 1.0 - also has more search options.
tag_for_keyword = pydicom.datadict.tag_for_name


# test decorator that skips test if dicom not available.
dicom_test = np.testing.dec.skipif(not have_dicom,
Expand Down