Skip to content

Conversation

@jamesobutler
Copy link
Collaborator

@jamesobutler jamesobutler commented Nov 13, 2025

This closes #119.

Pydicom warning:
get_frame_offsets is deprecated and will be removed in v4.0

See instructions at https://github.com/pydicom/pydicom/blob/main/doc/release_notes/v3.0.0.rst that pydicom get_frame_offsets usage should be replaced with parse_basic_offsets.

This PR maintains compatibility with pydicom >=2.2 which is supported per the pyproject.toml:

"pydicom>=2.2",

I discovered this deprecation usage while using pydicom 3 and latest dicomweb-client in the context of 3D Slicer as part of updating python packages to latest (Slicer/Slicer#8844).

cc: @fedorov

@fedorov
Copy link
Member

fedorov commented Nov 13, 2025

@jamesobutler thank you so much for the contribution!

I asked Copilot about the failing test, and this was the (most relevant, I think, part of the) response:

This is a static type-checker error (mypy), not a runtime ImportError. The code does try/except ImportError but mypy still resolves the attribute from the pydicom stubs used in CI and complains when it can't find parse_basic_offsets in those stubs.

Replace the direct conditional import with a safe getattr-based resolution. This avoids mypy trying to validate module attributes at import time and keeps runtime compatibility with both older and newer pydicom.

from typing import Any, Callable, Optional
import pydicom.encaps as _pydicom_encaps  # type: ignore

# Resolve functions dynamically (use getattr so mypy does not require the attribute in stubs)
_parse_basic_offsets: Optional[Callable[..., Any]] = getattr(
    _pydicom_encaps, 'parse_basic_offsets', None
)
_get_frame_offsets: Optional[Callable[..., Any]] = getattr(
    _pydicom_encaps, 'get_frame_offsets', None
)

if _parse_basic_offsets is not None:
    parse_basic_offsets = _parse_basic_offsets  # type: ignore
    _use_parse_basic_offsets = True
elif _get_frame_offsets is not None:
    get_frame_offsets = _get_frame_offsets  # type: ignore
    _use_parse_basic_offsets = False
else:
    # Should not happen for a standard pydicom install. Make the error explicit.
    raise ImportError(
        'Neither parse_basic_offsets nor get_frame_offsets available in pydicom.encaps'
    )

Or we could just require latest pydicom?

@pieper @CPBridge what would you recommend?

@jamesobutler jamesobutler force-pushed the pydicom-deprecation-replace branch from d01e29c to 9e866aa Compare November 13, 2025 03:34
@jamesobutler
Copy link
Collaborator Author

jamesobutler commented Nov 13, 2025

The mypy check should no longer fail following dropping of end-of-life Python 3.9. The Python 3.9 CI install of the application is pulling pydicom 2.4.4 because pydicom 3 requires Python 3.10+. I have added testing for Python 3.14 following the drop of Python 3.9. Whenever dicomweb-client last dropped python version support, the justification was because they were end-of-life versions as well (see #113).

I would recommend a v0.61.0 release considering the python requirements will have changed. Similarly in dicomweb-client v0.60.0 there was a bump in the python requirement.

@CPBridge
Copy link
Collaborator

CPBridge commented Nov 13, 2025

I do not think it's a good idea to try and support pydicom across the major version change 2.x to 3.x. I would favour requiring pydicom>=3.0.0 and removing the import hackery

@CPBridge
Copy link
Collaborator

I am also on board with dropping 3.9 support now that it is EOL

@CPBridge
Copy link
Collaborator

@jamesobutler I merged #121, could you please remove those changes from this PR

@jamesobutler
Copy link
Collaborator Author

Yes, I'll rebase this branch upon integration of #122.

@fedorov
Copy link
Member

fedorov commented Nov 13, 2025

I would favour requiring pydicom>=3.0.0 and removing the import hackery

I was thinking the same, but wanted to hear from Chris or Steve, since they have a lot more experience with python.
@jamesobutler let's do pydicom>=3.0.0!

@jamesobutler jamesobutler force-pushed the pydicom-deprecation-replace branch from aeeefd2 to 91f3c3d Compare November 13, 2025 15:45
@jamesobutler jamesobutler force-pushed the pydicom-deprecation-replace branch from 2ac8e62 to b4af343 Compare November 13, 2025 15:48
@sonarqubecloud
Copy link

@jamesobutler
Copy link
Collaborator Author

Todo:

  • Handle addition deprecations when using pydicom >=3 such as DeprecationWarning: 'Dataset.is_little_endian' will be removed in v4.0, set the Transfer Syntax UID or use the 'little_endian' argument with Dataset.save_as() or dcmwrite() instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pydicom get_frame_offsets is deprecated

3 participants