pydicom / pynetdicom

A Python implementation of the DICOM networking protocol
https://pydicom.github.io/pynetdicom
MIT License
513 stars 180 forks source link

Add constants for status values #454

Closed sloria closed 4 years ago

sloria commented 4 years ago

Is your feature request related to a problem? Please describe.

It would be convenient to have constants for the various status values.

Describe the solution you'd like

Ideally, there would be an IntEnum with the valid service statuses. I imagine it'd look similar to the http.HTTPStatus from the stdlib.

>>> from pynetdicom.status import Status
>>> Status.FAILURE
>>> <Status.FAILURE: 0xA700>

Describe alternatives you've considered

You could also use an enum of tuples or sets, so you could have e.g.

>>> from pynetdicom.status import Status
>>> assert Status.PENDING == {0xFF00, 0xFF01}

But this isn't clearly better and is less flexible.

Additional context

Might want to add this after Python 2 is dropped so you don't have to conditionally install an enum backport.

scaramallion commented 4 years ago

Would that support having multiple values for the same status category? For example, 0xA700 is failure, but so is any value in the range 0xC000 to 0xCFFF. Or are you more interested in knowing the category of a particular status value (similar to the status.code_to_category() function)? It's also possible for the status value to not be one from any of the the service classes (if you defined a private service class, for example, you may use your own range of statuses, or it could just be non-conformant).

Different service classes can also have different meanings for the same status value, although the category remains the same). Not sure if that matters though, presumably you could lookup the meaning via the service class (or status module).

sloria commented 4 years ago

Would that support having multiple values for the same status category?

I alluded to that in the "alternatives" section above, but I don't think it's necessary. Users can use the existing status.code_to_category to check the category.

I would just like to avoid having to use magic values in my SCP code. Here's what I'm currently doing:


from enum import IntEnum

class Status(IntEnum):
    CANCEL = 0xFE00
    PENDING = 0xFF00
    PENDING_WITH_WARNING = 0xFF01

# -----------------------------------------------------------------------------

MODALITY = "US"

def handle_find(event):
    """Handle a C-FIND request event."""
    app.logger.info("received C-FIND request for worklist id=%s", worklist.id)
    items = event.assoc._worklist.items

    for item in items:
        # Check if C-CANCEL has been received
        if event.is_cancelled:
            yield (Status.CANCEL, None)
            return
        identifier = to_identifier(item)
        # Pending
        yield (Status.PENDING, identifier)
scaramallion commented 4 years ago

I think this is a good idea, but including all the status codes is impractical so I decided that defaulting to a small set of statuses and allowing the user to add extra constants is the way to go.

from pynetdicom.status import Status

# Customise the class
Status.add('UNABLE_TO_PROCESS', 0xC000)

def handle_store(event):
    try:
        event.dataset.save_as('temp.dcm')
    except:
        return Status.UNABLE_TO_PROCESS

    return Status.SUCCESS

Documentation here

sloria commented 4 years ago

Great! Thanks for implementing this