Skip to content
This repository has been archived by the owner on Jun 18, 2024. It is now read-only.

Initial implementation #1

Merged
merged 33 commits into from
Dec 10, 2015
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6cb67fc
Add skeleton created by XBlock SDK.
itsjeyd Oct 27, 2015
9a9a324
Allow to display and interact with exercises in the LMS.
itsjeyd Oct 30, 2015
dd06d81
Add support for exercise descriptions.
itsjeyd Oct 30, 2015
aa7c134
Add grading functionality.
itsjeyd Oct 30, 2015
c19b0e4
Save and restore state.
itsjeyd Oct 30, 2015
6c1b897
Clean up.
itsjeyd Oct 30, 2015
6b87e0a
Address review comments.
itsjeyd Nov 2, 2015
c71e96b
Configure Travis.
itsjeyd Nov 2, 2015
a838256
Port tests.
itsjeyd Nov 2, 2015
273232b
Fix PEP8 and pylint issues.
itsjeyd Nov 2, 2015
06d0cbb
Render template for board and menu on the server.
itsjeyd Nov 3, 2015
fbc4ea4
Add workbench scenario.
itsjeyd Nov 3, 2015
06eacfc
Address review comments.
itsjeyd Nov 4, 2015
deac8d0
Add integration tests.
itsjeyd Nov 10, 2015
bc15783
Update test-requirements.
itsjeyd Nov 10, 2015
a67ccc1
Address review comments.
itsjeyd Nov 16, 2015
918ed51
Move controls and vector properties above board.
itsjeyd Nov 20, 2015
b574ec3
Address review comments.
itsjeyd Nov 21, 2015
708b2cc
Address review comments (i18n).
itsjeyd Nov 21, 2015
f19ab78
WYSIWYG functionality for Studio, Part 1: Add support for definining
itsjeyd Nov 18, 2015
c3a098d
Address review comments.
itsjeyd Nov 23, 2015
d2fe045
Make LMS and Studio functionality accessible.
itsjeyd Nov 24, 2015
d2d0e84
Studio: Make exercise preview render correctly on page load.
itsjeyd Nov 25, 2015
224391a
Address review comments.
itsjeyd Nov 26, 2015
602d021
WYSIWYG functionality for Studio, Part 2:
itsjeyd Nov 25, 2015
9e6e001
Address review comments.
itsjeyd Nov 30, 2015
dcf0840
Address review comments.
itsjeyd Dec 1, 2015
792d4ba
Remove :hover effect from disabled buttons.
itsjeyd Dec 2, 2015
d2a9f6e
Address upstream review comments.
itsjeyd Dec 4, 2015
0d4a06a
Address upstream review comments.
itsjeyd Dec 7, 2015
645115f
Address upstream review comments.
itsjeyd Dec 8, 2015
c03551d
Address upstream review comments.
itsjeyd Dec 9, 2015
461371b
Address upstream review comments.
itsjeyd Dec 9, 2015
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
__pycache__/
*.py[cod]
vectordraw_xblock.egg-info/
39 changes: 39 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
"""Setup for vectordraw XBlock."""

import os
from setuptools import setup


def package_data(pkg, roots):
"""Generic function to find package_data.

All of the files under each of the `roots` will be declared as package
data for package `pkg`.

"""
data = []
for root in roots:
for dirname, _, files in os.walk(os.path.join(pkg, root)):
for fname in files:
data.append(os.path.relpath(os.path.join(dirname, fname), pkg))

return {pkg: data}


setup(
name='vectordraw-xblock',
version='0.1',
description='vectordraw XBlock', # TODO: write a better description.
packages=[
'vectordraw',
],
install_requires=[
'XBlock',
],
entry_points={
'xblock.v1': [
'vectordraw = vectordraw:VectorDrawXBlock',
]
},
package_data=package_data("vectordraw", ["static", "public"]),
)
1 change: 1 addition & 0 deletions vectordraw/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .vectordraw import VectorDrawXBlock
229 changes: 229 additions & 0 deletions vectordraw/grader.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
import inspect
import json
import logging
import math


log = logging.getLogger(__name__)


## Built-in check functions

def _errmsg(default_message, check, vectors):
template = check.get('errmsg', default_message)
vec = vectors[check['vector']]
return template.format(name=vec.name,
tail_x=vec.tail.x,
tail_y=vec.tail.y,
tip_x=vec.tip.x,
tip_y=vec.tip.y,
length=vec.length,
angle=vec.angle)

def _errmsg_point(default_message, check, point):
template = check.get('errmsg', default_message)
return template.format(name=check['point'], x=point.x, y=point.y)

def check_presence(check, vectors):
if check['vector'] not in vectors:
errmsg = check.get('errmsg', 'You need to use the {name} vector.')
return errmsg.format(name=check['vector'])

def check_tail(check, vectors):
vec = vectors[check['vector']]
tolerance = check.get('tolerance', 1.0)
expected = check['expected']
dist = math.hypot(expected[0] - vec.tail.x, expected[1] - vec.tail.y)
if dist > tolerance:
return _errmsg('Vector {name} does not start at correct point.', check, vectors)

def check_tip(check, vectors):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function has a great similarity to check_tail. Is there a way to share code between the two?

vec = vectors[check['vector']]
tolerance = check.get('tolerance', 1.0)
expected = check['expected']
dist = math.hypot(expected[0] - vec.tip.x, expected[1] - vec.tip.y)
if dist > tolerance:
return _errmsg('Vector {name} does not end at correct point.', check, vectors)

def _check_coordinate(check, coord):
tolerance = check.get('tolerance', 1.0)
expected = check['expected']
return abs(expected - coord) > tolerance

def check_tail_x(check, vectors):
vec = vectors[check['vector']]
if _check_coordinate(check, vec.tail.x):
return _errmsg('Vector {name} does not start at correct point.', check, vectors)

def check_tail_y(check, vectors):
vec = vectors[check['vector']]
if _check_coordinate(check, vec.tail.y):
return _errmsg('Vector {name} does not start at correct point.', check, vectors)

def check_tip_x(check, vectors):
vec = vectors[check['vector']]
if _check_coordinate(check, vec.tip.x):
return _errmsg('Vector {name} does not end at correct point.', check, vectors)

def check_tip_y(check, vectors):
vec = vectors[check['vector']]
if _check_coordinate(check, vec.tip.y):
return _errmsg('Vector {name} does not end at correct point.', check, vectors)

def _coord_delta(expected, actual):
if expected == '_':
return 0
else:
return expected - actual

def _coords_within_tolerance(vec, expected, tolerance):
for expected_coords, vec_coords in ([expected[0], vec.tail], [expected[1], vec.tip]):
delta_x = _coord_delta(expected_coords[0], vec_coords.x)
delta_y = _coord_delta(expected_coords[1], vec_coords.y)
if math.hypot(delta_x, delta_y) > tolerance:
return False
return True

def check_coords(check, vectors):
vec = vectors[check['vector']]
expected = check['expected']
tolerance = check.get('tolerance', 1.0)
if not _coords_within_tolerance(vec, expected, tolerance):
return _errmsg('Vector {name} coordinates are not correct.', check, vectors)

def check_segment_coords(check, vectors):
vec = vectors[check['vector']]
expected = check['expected']
tolerance = check.get('tolerance', 1.0)
if not (_coords_within_tolerance(vec, expected, tolerance) or
_coords_within_tolerance(vec.opposite(), expected, tolerance)):
return _errmsg('Segment {name} coordinates are not correct.', check, vectors)

def check_length(check, vectors):
vec = vectors[check['vector']]
tolerance = check.get('tolerance', 1.0)
if abs(vec.length - check['expected']) > tolerance:
return _errmsg('The length of {name} is incorrect. Your length: {length:.1f}', check, vectors)

def _angle_within_tolerance(vec, expected, tolerance):
# Calculate angle between vec and identity vector with expected angle
# using the formula:
# angle = acos((A . B) / len(A)*len(B))
x = vec.tip.x - vec.tail.x
y = vec.tip.y - vec.tail.y
dot_product = x * math.cos(expected) + y * math.sin(expected)
angle = math.degrees(math.acos(dot_product / vec.length))
return abs(angle) <= tolerance

def check_angle(check, vectors):
vec = vectors[check['vector']]
tolerance = check.get('tolerance', 2.0)
expected = math.radians(check['expected'])
if not _angle_within_tolerance(vec, expected, tolerance):
return _errmsg('The angle of {name} is incorrect. Your angle: {angle:.1f}', check, vectors)

def check_segment_angle(check, vectors):
# Segments are not directed, so we must check the angle between the segment and
# the vector that represents it, as well as its opposite vector.
vec = vectors[check['vector']]
tolerance = check.get('tolerance', 2.0)
expected = math.radians(check['expected'])
if not (_angle_within_tolerance(vec, expected, tolerance) or
_angle_within_tolerance(vec.opposite(), expected, tolerance)):
return _errmsg('The angle of {name} is incorrect. Your angle: {angle:.1f}', check, vectors)

def _dist_line_point(line, point):
# Return the distance between the given line and point. The line is passed in as a Vector
# instance, the point as a Point instance.
direction_x = line.tip.x - line.tail.x
direction_y = line.tip.y - line.tail.y
determinant = (point.x - line.tail.x) * direction_y - (point.y - line.tail.y) * direction_x
return abs(determinant) / math.hypot(direction_x, direction_y)

def check_points_on_line(check, vectors):
line = vectors[check['vector']]
tolerance = check.get('tolerance', 1.0)
points = check.get('expected')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like 'expected' is a required key, so use check['expected']

for point in points:
point = Point(point[0], point[1])
if _dist_line_point(line, point) > tolerance:
return _errmsg('The line {name} does not pass through the correct points.', check, vectors)

def check_point_coords(check, points):
point = points[check['point']]
tolerance = check.get('tolerance', 1.0)
expected = check.get('expected')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like 'expected' is a required key, so use check['expected']

dist = math.hypot(expected[0] - point.x, expected[1] - point.y)
if dist > tolerance:
return _errmsg_point('Point {name} is not at the correct location.', check, point)

class Point(object):
def __init__(self, x, y):
self.x = x
self.y = y

class Vector(object):
def __init__(self, name, x1, y1, x2, y2):
self.name = name
self.tail = Point(x1, y1)
self.tip = Point(x2, y2)
self.length = math.hypot(x2 - x1, y2 - y1)
angle = math.degrees(math.atan2(y2 - y1, x2 - x1))
if angle < 0:
angle += 360
self.angle = angle

def opposite(self):
return Vector(self.name, self.tip.x, self.tip.y, self.tail.x, self.tail.y)

class Grader(object):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me like this Grader class exists for one reason only: its grade() method. Why define the class at all? Why not just pull the grade() method out into a standalone function, and call that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@singingwolfboy You're right about the fact that the grade method is the only thing being called from outside. But I think it makes sense to encapsulate the grading logic in a class, because otherwise we'd end up with either one long function that has all of the logic from the Grader class, or multiple functions that you wouldn't know are related just by looking at them (i.e., you'd have to read the code in more detail to understand the relationship between the functions).

check_registry = {
'presence': check_presence,
'tail': check_tail,
'tip': check_tip,
'tail_x': check_tail_x,
'tail_y': check_tail_y,
'tip_x': check_tip_x,
'tip_y': check_tip_y,
'coords': check_coords,
'length': check_length,
'angle': check_angle,
'segment_angle': check_segment_angle,
'segment_coords': check_segment_coords,
'points_on_line': check_points_on_line,
'point_coords': check_point_coords,
}

def __init__(self, success_message='Test passed', custom_checks=None):
self.success_message = success_message
if custom_checks:
self.check_registry.update(custom_checks)

def grade(self, answer):
check_data = dict(
vectors=self._get_vectors(answer),
points=self._get_points(answer),
)
for check in answer['checks']:
check_data['check'] = check
check_fn = self.check_registry[check['check']]
args = [check_data[arg] for arg in inspect.getargspec(check_fn).args]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed this hack I introduced when adding points as a problem type. It's main purpose was to stay backwards-compatible with existing external graders. We might now be able to simply pass check, vectors, points as arguments to all graders. (Up to you whether you want to change this. it would require editing the parameter lists of all functions in this file, and updaing all the tests.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, another way to do this would be to declare all checks with a **unused argument at the end, and then invoke it with check_fn(**check_data).

result = check_fn(*args)
if result:
return {'ok': False, 'msg': result}
return {'ok': True, 'msg': self.success_message}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return type seems wrong to me. Python allows functions to return multiple values. If you want the function to report both whether the student's answer was correct or not, and a message to report to the student, you can just return those two values, rather than combining them into a dictionary. I also don't like the word ok in this context: it's ambiguous whether you mean "this code ran successfully vs. failed" or if you mean "the student's response is marked as passing vs failing".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@singingwolfboy Definitely agree on ok not being the best choice in this context; I changed it to correct to make it clear that it refers to the student's answer being marked as correct vs. incorrect. In terms of the return type I think it makes sense for the grader to return the result in a format that we can use as is to both persist information about the most recent result obtained by the student (in the JSON handler), and to display appropriate feedback to the student in the LMS (cf. updateStatus). (In other words, we'd be constructing that dict anyway if the grader returned separate values.)


def cfn(self, e, ans):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this function is needed for the XBlock version – it looks quite JSInput-specific.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarnach Ah, yes - missed that somehow when I was cleaning up. Thanks!

answer = json.loads(json.loads(ans)['answer'])
return self.grade(answer)

def _get_vectors(self, answer):
vectors = {}
for name, props in answer['vectors'].iteritems():
tail = props['tail']
tip = props['tip']
vectors[name] = Vector(name, tail[0], tail[1], tip[0], tip[1])
return vectors

def _get_points(self, answer):
return {name: Point(*coords) for name, coords in answer['points'].iteritems()}
113 changes: 113 additions & 0 deletions vectordraw/static/css/vectordraw.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/* CSS for VectorDrawXBlock */

.vectordraw_block,
.vectordraw_block #vectordraw {
display: inline-block;
}

.vectordraw_block .vectordraw-description,
.vectordraw_block #vectordraw,
.vectordraw_block .vectordraw-status {
margin-bottom: 1.5em;
}

.vectordraw_block .jxgboard {
float: left;
border: 2px solid #1f628d;
}

.vectordraw_block .jxgboard .JXGtext {
pointer-events: none; /* prevents cursor from turning into caret when over a label */
}

.vectordraw_block .menu {
float: left;
width: 160px;
margin-left: 15px;
}

.vectordraw_block .menu .controls {
margin-bottom: 20px;
font-size: 0;
}

.vectordraw_block .menu .controls select {
width: 160px;
margin-bottom: 8px;
font-size: 18px;
}

.vectordraw_block .menu .controls button {
display: inline-block;
background-color: #3498db;
border-radius: 5px;
box-shadow: 0 1px 3px #666;
color: #1f628d;
font-size: 14px;
padding: 5px 10px 5px 10px;
margin: 4px 0;
border: 2px solid #1f628d;
text-decoration: none;
width: 160px;
}

.vectordraw_block .menu .controls button:hover {
background: #3cb0fd;
background-image: -webkit-linear-gradient(top, #3cb0fd, #3498db);
background-image: -moz-linear-gradient(top, #3cb0fd, #3498db);
background-image: -ms-linear-gradient(top, #3cb0fd, #3498db);
background-image: -o-linear-gradient(top, #3cb0fd, #3498db);
background-image: linear-gradient(to bottom, #3cb0fd, #3498db);
text-decoration: none;
}

vectordraw_block .menu .controls button.undo,
vectordraw_block .menu .controls button.redo {
width: 78px;
}

.vectordraw_block .menu .controls button.undo {
margin-right: 4px;
}

.vectordraw_block .menu .vector-properties {
padding: 10px;
border: 1px solid #1f628d;
font-size: 16px;
line-height: 1.25;
}

.vectordraw_block .menu .vector-properties h3 {
font-size: 16px;
margin: 0 0 5px;
}

.vectordraw_block .menu .vector-properties .vector-prop-bold {
font-weight: bold;
}

.vectordraw_block .menu .vector-prop-slope {
display: none;
}

.vectordraw_block .action button {
height: 40px;
margin-right: 10px;
font-weight: 600;
text-transform: uppercase;
}

.vectordraw_block .vectordraw-status {
display: inline-block;
width: 100%;
}

.vectordraw_block .checkmark-correct {
font-size: 22pt;
color: #629b2b;
}

.vectordraw_block .checkmark-incorrect {
font-size: 22pt;
color: #ff0000;
}
Loading