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 support for modalities, add classification to files #649

Closed
wants to merge 5 commits into from

Conversation

nagem
Copy link
Contributor

@nagem nagem commented Feb 2, 2017

New endpoints:

GET /api/modalities - GET list of all modalities and their allowable classifications.
GET /api/modalities/<modality_name> - GET a specific modality
POST /api/modalities - Add a new modality. Required: _id (readable name) and classification, a map of allowable classifications. Requires superuser. Example request:

POST /api/modalities HTTP/1.1
Content-Type: application/json
{
    "_id": "MR",
    "classifications": {
        "Contrast": ["B0", "B1", "T1", "T2", "T2*", "PD", "MT", "ASL", "Perfusion", "Diffusion", "Spectroscopy", "Susceptibility", "Velocity", "Fingerprinting"],
        "Intent": ["Structural", "Functional", "Localizer", "Shim", "Calibration"],
        "Features": ["Quantitative", "Multi-Shell", "Multi-Echo", "Multi-Flip", "Multi-Band", "Steady-State", "3D", "Compressed-Sensing", "Eddy-Current-Corrected", "Fieldmap-Corrected", "Gradient-Unwarped", "Motion-Corrected", "Physio-Corrected"]
    }
}

PUT /api/modalities/<modality_name> - REPLACE a specific modality. Requires superuser.
DELETE /api/modalities/<modality_name> - DELETE a specific modality. Requires superuser.

Certain file attributes can now be modified: classification, info, and modality.

PUT /api/<cont_name>/<cont_id>/files/<files_name> - UPDATE an existing file. Optional flag of replace_fields=true will replace the dict fields rather than update them.

Review Checklist

  • Tests were added to cover all code changes
  • Documentation was added / updated
  • Code and tests follow standards in CONTRIBUTING.md

@nagem
Copy link
Contributor Author

nagem commented Feb 2, 2017

If you would like to "unset" any of the fields, set info and classification to {} and modality to null. These are the expected and allowed null types.

@kofalt
Copy link
Contributor

kofalt commented Feb 13, 2017

This will need to adjust gear rules and potentially affect what ops (ping @ryansanford et al) does on system start / upgrade.

Specifically, the file.measurements rule type will be affected. Possibly others, @nagem?

@kofalt
Copy link
Contributor

kofalt commented Feb 17, 2017

@gsfr We have an issue with this new model, where we cannot create gear manifest constraints that reference these fields.

@nagem Linked me this example of a file object, post update:

{
	"name": "example_file.txt",
	"modality": "MR",
	"classification": {
		"intent": ["Functional", "Localizer"],
                "custom": ["custom", "classification", "markers"]
	}
}

@jenreiter is building a gear that would want to specify that an input takes one of two intents, and my understanding is that we cannot write a schema that would create this constraint.

This issue is tracked upstream by json-schema-org/json-schema-spec#63 and more broadly json-schema-org/json-schema-spec#170 --> json-schema-org/json-schema-spec#206.

My understanding is that this isn't a new issue caused by this specific update, but it's worth noting that we can't create a manifest that talks about every section of the file object, which is a big deal. Thoughts?

@kofalt kofalt changed the title Classification Add support for modalities, add classification to files Feb 17, 2017
@ryansanford
Copy link
Contributor

@gsfr Is "modality" the correct term to use here. Much like measurement vs classification, I'm concerned that "modality" is to narrow. I'm not aware of that term outside medical acquisition devices.

@ryansanford
Copy link
Contributor

@nagem
Check in with @gsfr tomorrow regarding the concerned you raised about inability to use mongo aggregation for filtering now that classification is within a list of lists.

@nagem
Copy link
Contributor Author

nagem commented May 12, 2017

As part of this PR, we should add bootstrapping logic to load in a "default" modality and it's classification.

Also, filetypes.json should be stored in the db as well.

@kofalt
Copy link
Contributor

kofalt commented May 12, 2017

When this is closer to merge, we'll need to discuss what changes the SDK will need to make.

@nagem
Copy link
Contributor Author

nagem commented Nov 27, 2017

Closing, replaced by #1010

@nagem nagem closed this Nov 27, 2017
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.

4 participants