-
Notifications
You must be signed in to change notification settings - Fork 9
updated models to support background images #100
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
Conversation
This was mainly added so that a background image could be applied to a protocol/input for the recently added cavas attribute at ReproNim/reproschema-ui#363 Potentially allows for background use in other activities b/c it was easier to minimally implement and/or one could add a background image/logo or something for their own protocol.
for more information, see https://pre-commit.ci
…om:ReproNim/reproschema-py into allow-background-images-for-canvas-drawing
@yibeichan @djarecka could someone take a look at this? Also, I think I'll need to git tag it with a new version to trigger a build on pypi correct? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR looks very clean to me. Do you need to also update some docs to point out backgroundImage
as a newly existent thing or will it auto-generate in sphinx reference docs or some such thing? Also, I can't guarantee myself this is satisfactory to the reproschema coding styling, but if it passes CI I suspect it's good. Aside from all that, given the fact you've tested this and it works, I believe you've got a great PR here.
@ericearl I think that once this piece is in place along with the other PR's we'll be updating the docs at repronim/reproschema. The only thing I'm uncertain about is whether to call it canvas or something else. Maybe it's clear enough that this is used to capture drawing inputs, but (for sure) we're going to want to update the documentation. I'm thinking that those updates might be a great exercise (not just for this new field) for you, me, and Paul here at the NIMH. Especially since we're all fresh to this project. |
@djarecka this one looks good to me do you have anything specific regarding the model that we need to pay extra attention to or we can merge this one? |
hi, sorry for the delays, but this is not the repository to change the model. The reproschema model is here: https://github.com/ReproNim/reproschema I don't see the changes there, am I missing something @yibeichan ? |
@djarecka this is for the |
you are modifying the model, by adding I can help with the linkml model and creating a new release if this new property is needed. We can also come back to the discussion, if indeed the model changes is needed and we should have both @satra - there is no really good description of |
@bendhouseart - I've read some of the conversations on various repos again. In some you mentioned adding just a static image/logo, and this I believe you can easily use the existing In other places, you mentioned drawing on the background, and this is the case that I would try to understand more. Do some of your |
@djarecka This was a discovered need for the Montreal Cognitive Assessment (MoCA), specifically. The assessment question is called "Alternating Trail Making" and you can read about it here: https://www.smchealth.org/sites/main/files/file-attachments/moca-instructions-english_2010.pdf There's also some pictures here: https://geriatrictoolkit.missouri.edu/cog/MoCA-8.3-English-Test-2018-04.pdf
Yes.
I suspect @bendhouseart is fine with any name in the code/schema, but would just like this ability to exist in the model or schema. |
@ericearl - sounds good, let's add another option for item, and add additional property to the model, but this really should be done in the reproschema repository: https://github.com/ReproNim/reproschema. |
@bendhouseart , @ericearl - I've added a quick note on the model to the |
Yes! Lately I've been more focused on getting a dev environment working to better test out changes given that I'm only able to serve and test them by a series of ad-hoc scripts I've got, so thank you all for pushing this back up the priority list. This is the only feature lacking from reproschema for our lab; having it would enable us to transition entirely away from good ole paper.
I would say not so easily as it's been challenging just getting a dev environment working ;) From my perspective the path of least resistance would be to get the canvas feature live and working at -> https://www.repronim.org/reproschema-ui/#/ via this update and the successful merging of the added canvas input to the demo protocol. Concerning the property/selection of All that said I'll admit one can make just as convincing of an argument about the existing |
@bendhouseart - I've started PR in the model repo: ReproNim/reproschema#543 This is the pydantic version generated from new yaml, so you can check: https://github.com/djarecka/reproschema-py/blob/tmp_new_model/reproschema/models/model.py There is new formatting in newer linkml, so you will see more changes. Let me know if this will work. |
Also, right now, we don't check |
we can close this one since we should fix everything in ReproNim/reproschema#543 |
This was mainly added so that a background image could be applied to a protocol/input for the recently added cavas attribute at ReproNim/reproschema-ui#363
Potentially allows for background use in other activities b/c it was easier to minimally implement and/or one could add a background image/logo or something for their own protocol.
Helps resolve failing CI at ReproNim/demo-protocol#33