ReproNim / reproschema-py

Apache License 2.0

2 stars 8 forks source link

Add tests to create items, activities and protocols #18

Closed Remi-Gau closed 2 years ago

Remi-Gau commented 3 years ago

Gave a go at writing "some" tests and making sure that the different classes for schema creation sort of work well together.

Not sure if I have over-engineered some things or not and it is very likely that those classes and methods have been tailored for my specific needs.

Happy to get feedback and pointers to make them more usable by others.

satra commented 3 years ago

@Remi-Gau - are you coordinating with @sanuann's work on proper Python classes for the operations based on this doc here: https://docs.google.com/document/d/1dOyb5jy1BYY9rnplsTrg5F44sPYB5O5MTPBmd2kscCs/edit

Remi-Gau commented 3 years ago

oh dear... I am afraid I did not.

Had not realized she was on it... I had simply forgotten about that doc too...

:facepalm:

@sanuann Am not sure how far you got on this front. Let me know if I'd rather close this PR or if there is something to salvage from this.

sanuann commented 3 years ago

oh dear... I am afraid I did not.

Had not realized she was on it... I had simply forgotten about that doc too...

🤦

@sanuann Am not sure how far you got on this front. Let me know if I'd rather close this PR or if there is something to salvage from this.

as per our conversation today, let's catch up at the end of the week.

Remi-Gau commented 3 years ago

@sanuann I have commented the code and test and sort of highlighted the things that are already weird and wrong in the code. There are many things that should already be refactored and the API should be clarified especially for items.

I suspect that we will need to create a UI class and change the implementation of the response option class.

In terms of where to start. I would suggest the following reading order:

test for protocol
test for activities
code for protocol
code for activities
code for base class
test for items
code for items

Feel free to review and comment (preferably via github code reviews :-) )

Remi-Gau commented 3 years ago

oh and I can take care of some of the refactoring but not before next week.

Remi-Gau commented 3 years ago

[ ] the visibility "status" of a given item is not easily updatable either at the item level or the activity level, this will probably require some new methods

Remi-Gau commented 3 years ago

I will progressively add more tests to generate each inputType of items and wlll update the code accordingly. I don' t think I will do it in one big push though. But this way you can follow along "commit by commit"

Remi-Gau commented 2 years ago

will close this PR and open one directly on master to get a cleaner diff