-
Notifications
You must be signed in to change notification settings - Fork 42
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
Content classification nb (Ch 1) plus question-bank data model #44
Conversation
Finished questions + answers + feedback for part 1, started working on questions for part 2.
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.
@khdebruijn great work! I have left two points of discussion. Can you comment on those before we continue merging? If you agree with my proposed changes and don't see any caveats for the current NB, I would be glad to implement it.
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.
@khdebruijn, what do you think about loading the questions from a JSON file?
Now it's for example:
question_data11 = {
"name": "Q1-1",
"question": "At what depth do earthquakes along divergent boundaries generally occur?",
"options": {
"a": "less than 33 km",
"b": "between 33 km and 74 km",
"c": "between 74 km and 140 km",
"d": "more than 140 km "
},
"answer": "a",
"feedback": {"correct": "Correct, earthquakes at diverging boundaries typically occur at shallower depths",
"incorrect": "Incorrect. Note that at diverging boundaries the tectonic plates move away from each other. Magma wells up to (relatively) shallow depths in order to fill the gap. This may cause earthquakes. Where are these earthquakes likeliest to occur?"}
}
pn.Column(MC(question_data11))
Instead we could do something like:
question_href = "https://questions/are/stored/here.json"
def load_question_data_from_href(href):
...
return data
# only has to be load once at the start of the NB
question_data = load_question_data_from_href(href)
q1 = MC(question_data["Q1-1"])
q2 = MC(question_data["Q1-2"])
pn.Column(q1, q2)
In this example the questions stored at https://questions/are/stored/here.json
simply has to be something like this:
{
"Q1-1": {
"name": "Q1-1",
"question": "At what depth do earthquakes along divergent boundaries generally occur?",
"options": {
"a": "less than 33 km",
"b": "between 33 km and 74 km",
"c": "between 74 km and 140 km",
"d": "more than 140 km "
},
"answer": "a",
"feedback": {
"correct": "Correct, earthquakes at diverging boundaries typically occur at shallower depths",
"incorrect": "Incorrect. Note that at diverging boundaries the tectonic plates move away from each other. Magma wells up to (relatively) shallow depths in order to fill the gap. This may cause earthquakes. Where are these earthquakes likeliest to occur?"
}
}
}
The advantage of this would be that it's easier for the teachers to edit the questions, while the students don't see all this content in the NB. Also, in the long run we can work towards a method where we load the question data in such way that the students don't get access to the answers.
What do you think? Was there a specific reason for adding the questions in the NB itself?
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.
Hi Floris,
I agree with you! I left the question content in the notebook so Stuart can easily review both the questions and the notebook content without having to look for the question data (once he gets to that). I am waiting for his feedback because for notebook 1 I developed a large part of the questions, answers, and feedback myself.
I was planning on moving the question_data variables to the initializer notebook once he had given me his feedback and made his own changes, and run that notebook at the top of the actual week 1 notebook in order to assign all the question_data variables.
I remember that you would rather convert the initializers into .py files, so in that case loading the questions from a question bank is likely more suitable.
Working with .json would be completely fine by me, in that case we just have to make sure that Stuart knows where he can find the questions to be reviewed!
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.
@khdebruijn, what do you think about changing the question classes so that you don't need to repeat the create_widgets()
and serve()
for all questions?
Currently it's like this:
def MC(question_data):
Q = MultipleChoiceQuestion(
question_name=question_data["name"],
question_text=question_data["question"],
question_options=question_data["options"],
question_answer=question_data["answer"],
question_feedback=question_data["feedback"]
)
Q.create_widgets()
return Q.serve()
def MS(question_data):
Q = MultipleSelectionQuestion(
question_name=question_data["name"],
question_text=question_data["question"],
question_options=question_data["options"],
question_answers=question_data["answers"],
question_feedback=question_data["feedback"]
)
Q.create_widgets()
return Q.serve()
def num(question_data):
Q = NumericQuestion(
question_name=question_data["name"],
question_text=question_data["question"],
question_answer=question_data["answer"],
question_feedback=question_data["feedback"],
precision=question_data["precision"]
)
Q.create_widgets()
return Q.serve()
def text(question_data):
Q = TextQuestion(
question_name=question_data["name"],
question_text=question_data["question"],
question_answer=question_data["answer"],
question_feedback=question_data["feedback"],
)
Q.create_widgets()
return Q.serve()
Instead I could change the class and add a make()
method to the class. Instead of initializing the MC and other question types in this notebook, you can then just import the class. So to get the same result you would do something like:
from coastal_dynamics.questions.multiple_choice import MultipleChoiceQuestion as mc
q1 = mc(question_data["Q1-1"]).make() # assuming question data is a dictionary with all required content
The make()
call will trigger the create_widgets()
and serve()
call. The advantage of this is that we only have to maintain this workflow at one place, where the class is defined. What do you think?
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.
That sounds good to me! Stuart will likely work on the week 8 and week 1 notebook tomorrow, so maybe we can wait for him so we don't get conflicts with the week 1 and week 8 notebooks?
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.
@khdebruijn, sorry I accidently edited your comment and I don't know how to revert it. Seems I can speak in your name haha.
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.
@khdebruijn, great! I'll edit the classes then. Also I'll prepare the questions in a seperate JSON file that I can test, but then we wait with the final implementation until Stuart is done.
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.
@khdebruijn, I have changed the classes a bit to reduce the repetition of code while making the questions for the NB. There is now a QuestionFactory that can make all four question types from a dictionary argument (question_data). This builds upon what you did in Week_1_Initialize.ipynb with the functions MS, mq, text, num
. Here is an example of how to use it:
import panel as pn
from coastal_dynamics.questions.factory import QuestionFactory as qf
question_data11 = {
"name": "Q1-1",
"type": "multiple_choice",
"question": "At what depth do earthquakes along divergent boundaries generally occur?",
"options": {
"a": "less than 33 km",
"b": "between 33 km and 74 km",
"c": "between 74 km and 140 km",
"d": "more than 140 km ",
},
"answer": "a",
"feedback": {
"correct": "Correct, earthquakes at diverging boundaries typically occur at shallower depths",
"incorrect": "Incorrect. Note that at diverging boundaries the tectonic plates move away from each other. Magma wells up to (relatively) shallow depths in order to fill the gap. This may cause earthquakes. Where are these earthquakes likeliest to occur?",
},
}
question_data12 = {
"name": "Q1-2",
"type": "multiple_selection",
"question": "Where do earthquakes with a depth > 250 km typically occur? Multiple answers may be correct.",
"options": {
"a": "wherever two oceanic plates converge",
"b": "at a boundary between an oceanic and continental plate",
"c": "at island arcs",
"d": "wherever two oceanic plates diverge",
},
"answers": ["a", "c"],
"feedback": {
"correct": "Correct, indeed very deep earthquakes occur mostly when two ocaenic plates converge, or at an island arc.",
"incorrect": "Incorrect. Deep earthquakes occur due to deep processes such as subduction. Due to subduction, island arcs may form (see Figure 2.9 in the book). Where does subduction typically occur?",
},
}
pn.extension()
q1 = qf(question_data11) # it should serve by default. If not change to `qf(question_data12).serve()`
q2 = qf(question_data12) # it should serve by default. If not change to `qf(question_data12).serve()`
pn.Column(q1, q2)
Probably the only difference you note is that the question_data
now also requires a "type" key which specifies the question type. The valid question types are one of ["multiple_choice", "multiple_selection", "text", "numeric"]
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.
@khdebruijn, I have made a template to make the questions from a JSON file that serves as our question bank. You can use the cd.QuestionFactory()
to make the questions from that file. You can find updated examples in this notebook: https://github.com/FlorisCalkoen/CoastalCodebook/blob/kevin/week1/cookbook/question_types.ipynb
And here is a file that can serve as a start for the questions in the coastal classification notebook: https://github.com/FlorisCalkoen/CoastalCodebook/blob/kevin/week1/notebooks/questions/01_coastal_classification.json Watch out for trailing comma's when converting Python dictionaries to JSON format! You will find out what I mean when you get an error that the JSON file is invalid.
Little extra explanation on why we are doing this. For now we can leave the questions in the github, but in the long run I'll move the JSON files with questions to a private cloud container, so then the students don't have access to the answers.
Hopefully this is a easier way to keep track of the questions. Also the notebooks will be less cluttered. Please let me know if you have doubts or other thoughts.
add panel to dev env
names of notebook week 1 (initializer as well) to match Floris' convention
Finished first draft of week 1 notebook. Still needs to be reviewed and checked. Also still need to move question data and functions to initializer, but will do that after review.