MRG: Prepare for pydicom 1.0

[effigies]

pydicom 1.0 will change the import name from dicom to pydicom, and move dicom.read_file to pydicom.dicomio.read_file. Adjust imports accordingly, and rename dicom to pydicom, when imported directly.

Closes #351

[effigies]

@ignatenkobrain Do you want to try this branch out?

[bcipolli]

This looks ok to me. My concern is: how are we going to test the two codepaths?

[effigies]

Until pydicom 1.0 is actually released, I'm not sure. I don't know if we can set up a Travis test to use a commit off Github. But there is a Travis test using the latest pydicom release, so we'll at least make sure we're not breaking anybody who's not bleeding edge.

I'm also okay with waiting to merge this until the 1.0 release of pydicom, in which case @ignatenkobrain can test this patch (which gives us some assurance that it's correct) and include it in his package. That way the patch is at least the one we expect to go in, so he's not stuck with an orphaned patch.

[arokem]

Would pip installing from github work for this?

http://stackoverflow.com/questions/13685920/install-specific-git-commit-with-pip

On Fri, Nov 6, 2015 at 7:49 AM, Chris Markiewicz [email protected]
wrote:

Until pydicom 1.0 is actually released, I'm not sure. I don't know if we
can set up a Travis test to use a commit off Github. But there is a Travis
test using the latest pydicom release, so we'll at least make sure we're
not breaking anybody who's not bleeding edge.

I'm also okay with waiting to merge this until the 1.0 release of pydicom,
in which case @ignatenkobrain https://github.com/ignatenkobrain can
test this patch (which gives us some assurance that it's correct) and
include it in his package. That way the patch is at least the one we expect
to go in, so he's not stuck with an orphaned patch.

—
Reply to this email directly or view it on GitHub
#379 (comment).

[matthew-brett]

Some test failures...

[effigies]

Thanks. Scanning today, so intermittent ability to check.

[effigies]

This is a little ugly, reflecting the need to discover two different pydicom versions. The try/except/else allows us to detect a failure to import pydicom separately from a failure to find pydicom.dicomio.read_file.

[effigies]

Tests passing. Code review appreciated.

[bcipolli]

Is it worth importing InvalidDicomError here, like you did with read_file? I know there's no change in the package path, but would be more consistent with the others (including Dataset)

[bcipolli]

LGTM. Essentially all of the try..catch blocks are in test code, and I don't think it's worth any extra effort to try and make things "prettier". I also think there's no need for further code comments; the code is pretty clear about what changed, and the commits/ PR document the time & reason for the change well.

All that to, again, say: LGTM

[ignatenkobrain]

Hi folks, I want. I will test it during weekend.

[ignatenkobrain]

I have checked this with PR for 2.0.2. Fixes my issue and works perfectly! Would be great if this also will be part of 2.0.2 (but not required, I will backport this patch myself)

[effigies]

I'd like to put this into 2.0.2.

Squashed down to two commits. Ready to merge? @matthew-brett @bcipolli

[bcipolli]

👍 Took a final look, happy from my side

[matthew-brett]

Thanks for this - in it goes.