ENH: Allow specification of reference attribute for parameters of type file #8
Conversation
|
@jcfr can you please take a look at this. Should i allow the reference attributes for all external types here by checking for self.isExternalType() |
|
Hello, Can anyone give this a quick QA/merge? We are in need of this change for a downstream toolkit and would love to be able to install directly from the main repo. Thanks! |
|
Did you see commontk/CTK#623 ? The problem is that the semantics for the reference attribute are not clear. I can totally see your point, and am not opposed to the change, but apparently, the meaning of the attribute was never formalized and can be misunderstood. Maybe it would be better (and easier to agree on / get integrated into CTK and ctk-cli) to introduce a new attribute like source-image or so? |
|
I like the idea from Hans. On Mon, Aug 1, 2016 at 9:45 AM, Hans Meine [email protected] wrote:
|
|
Ah, I see. Yes, we are using it right now for a reference to the source image, but I think the intent was to keep it intentionally vague so that it could be used to store arbitrary provenance information. It's a difficult problem to try and enumerate the whole ontology surrounding provenance, so this field allows us to capture whatever we need and pass it in an opaque, application-specific way, and support future uses of it as well without having to modify the spec. I agree we should add documentation for the field before going any further with it. |
|
Yes, please. The main goal of the CLI spec and interface is to prevent "opaque, application-specific ways". ;-) I agree that the "whole ontology surrounding provenance" is hard to tackle, so let's try to start with very specific use cases. It seems that you have one, which is very good, because I could not get any definitive answer ("XY uses it for …" instead of just "I think I saw someone using it in the past") when I looked for an explanation of the reference attribute. :-) So, my suggestion is to start by enumerating very specific, single use cases, and later try to generalize, if possible at all. |
|
Ideally a general purpose library such as CTK could be applied to novel problems without requiring an upstream change for each new use case, which I think was the idea here. Using opaque data fields is not unprecedented for maximizing interoperability and generality of an API. A couple examples: OAuth with their For us, there is a lot of value in having a general purpose field like this, it could make it quite easy for us to use CTK in a wide variety of applications moving forward. At this time, there is not a lot of value for us in attempting to build formal provenance models. Of course, I am not a maintainer of this project, this is just my 2c as a user. 😄 |
|
I highly appreciate your input! (This is a community project, so your voice does not count less than others.) I think we agree, the question is just "how opaque" something is. For instance, I have started this ctk-cli project as part of trying to get CLIs integrated in MeVisLab (instead of Slicer). When doing so, it is very important to get an idea of how existing attributes are used in practice, and it becomes impossible to support them if there's no explanation at all. In this case, my question would be: Is there any value in supporting the attribute in different host environments? If the answer is "no", then one might go with an opaque value, but I would suspect (and hope) that it is possible to describe the use case and purpose of the attribute, such as to support its use in various host applications (Slicer vs. MeVisLab vs. MITK vs. nipype…). |
|
While i totally agree that it is important to document the semantics of the reference attribute, i feel for whichever reason due to which it is currently enabled for parameters of types - For example, can we dig out the reason to allow the specification of the reference attribute for parameters of type Maybe this generalizes to anything that CLIParameter.isExternalType returns True for except directory may be. |
|
That's exactly what I have been trying to do with commontk/CTK#623 . You're just the first actual user of this attribute (even in a new context) that I am getting a hold of. ;-) |
…eSystem for regions (#1) * Handle default value for region types correctly and expose coordinateSystem * Remove unnecessary changes
|
Any resolution on this? I have now also exposed the coordinateSystem attribute for region typed parameters. |
The addition of the reference attribute in ctk-cli is included in an unmerged PR: commontk/ctk-cli#8 HistomicsTK relies on a fork of ctk-cli for this addition. Rather than remove the feature all together, I have removed the test so that item_tasks doesn't rely on a fork. Instead, we will install the fork for HistomicsTK deployments.
|
Code-wise, this change looks trivial enough to be merged. What is the conclusion concerning the XSD / spec? |
The addition of the reference attribute in ctk-cli is included in an unmerged PR: commontk/ctk-cli#8 HistomicsTK relies on a fork of ctk-cli for this addition. Rather than remove the feature all together, I have removed the test so that item_tasks doesn't rely on a fork. Instead, we will install the fork for HistomicsTK deployments.
I have a use case wherein a CLI segments the nuclei in a histopathology image and generates an annotation file (json) containing bounding boxes of all nuclei.
The code that creates the web-front end for this CLI needs to know which image the generated annotation file belongs to, so it could ingest all the nuclei annotations/bounding-boxes into a database with key being the Image-Id.