Skip to content
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.

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

Add constants for status values #454

Closed
sloria opened this issue Feb 24, 2020 · 4 comments · Fixed by #464
Closed

Add constants for status values #454

sloria opened this issue Feb 24, 2020 · 4 comments · Fixed by #464

Comments

@sloria
Copy link
Contributor

sloria commented Feb 24, 2020

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
Copy link
Member

scaramallion commented Feb 24, 2020

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
Copy link
Contributor Author

sloria commented Feb 25, 2020

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
Copy link
Member

scaramallion commented Mar 6, 2020

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
Copy link
Contributor Author

sloria commented Mar 7, 2020

Great! Thanks for implementing this

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

Successfully merging a pull request may close this issue.

2 participants