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

#181 RadReport's template fields #185

Merged
merged 4 commits into from Oct 26, 2017

Conversation

Projects
None yet
3 participants
@vessemer
Copy link
Contributor

vessemer commented Oct 24, 2017

The models were adopted to suit the RSNA Radiology Reporting Templates for CT Lung Cancer Screening.

Description

All feilds were assigned to the Case model, except for those listed below which were assigned to the Nodule model:

  • diameter
  • appearance_feature
  • density_feature
  • image_no

Due to the symmetry of right and left pleural spaces, a new model PleuralSpace was created and it consists of:

  • effusion
  • calcification
  • thickening
  • pneumothorax

The Case model then receives two foreign keys for left and right pleural spaces.

Reference to official issue

This address #181

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@unique # ensures all variables are unique
@django_enum
class TypesOne(IntEnum):

This comment has been minimized.

@lamby

lamby Oct 24, 2017

Contributor

From experience I have a very strong preference for putting enumerations into a separate enums.py file alongside the models.py — could you try this approach and see how it looks/works for you? It might not be suitable, but definitely worth having a look at...

This comment has been minimized.

@vessemer

vessemer Oct 24, 2017

Contributor

Done: enum.py, looks much better now, thanks!

EXTENSIVE = 3

effusion = models.IntegerField(
choices=[(choice.value, choice.name.replace("_", " ")) for choice in TypesOne],

This comment has been minimized.

@lamby

lamby Oct 24, 2017

Contributor

Feels like we could abstact this :)

This comment has been minimized.

@vessemer

vessemer Oct 24, 2017

Contributor

Done, I've added the format_enum function inside the enum.py file.


coronary_calcification = models.IntegerField(
choices=[(choice.value, choice.name.replace("_", " ")) for choice in ShapeChoices],
default=ShapeChoices.NONE.value)

This comment has been minimized.

@lamby

lamby Oct 24, 2017

Contributor

I worry that we have too many nullable (or quasi-nullable with NONE enum value) here. These are pretty common sources of bugs. Can I suggest trying a "hanging table" approach and seeing how that feels? eg.

class Parent(models.Model):
    field_a = models.IntegerField()
    field_b = models.IntegerField()

class ExtraDataForParent(models.Model):
    parent = models.OneToOneField(Parent, related_name='extra_data', primary_key=True)
    field_c = models.IntegerField()
    field_d = models.IntegerField()

Note that crucially field_c and field_d are not nullable as you only create ExtraData models when you know you have both of those fields, so there is no possibility of having field_c but not field_d etc. etfc.

This comment has been minimized.

@vessemer

vessemer Oct 24, 2017

Contributor

Thank you, it's absolutely rational to decompose the models.

@vessemer vessemer force-pushed the vessemer:181_RadReport_template_fields branch from e9eac8b to 27df8b8 Oct 24, 2017

@vessemer vessemer force-pushed the vessemer:181_RadReport_template_fields branch from 27df8b8 to f0e52cb Oct 24, 2017

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Oct 25, 2017

Nice @vessemer. What do you think of the separate enum file approach? The only thing I would ask now is that you rename it to enums.py as Python already ships with an enum module so this clashes... *g*

@lamby lamby merged commit 6761064 into drivendataorg:master Oct 26, 2017

2 checks passed

concept-to-clinic/cla @vessemer has signed the CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Oct 26, 2017

Merged, many thanks!

@vessemer vessemer deleted the vessemer:181_RadReport_template_fields branch Oct 26, 2017

@isms

This comment has been minimized.

Copy link
Contributor

isms commented Oct 26, 2017

Awesome - thanks @vessemer 🎉

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