- fix README.md error

- refactor several functions in app.py and rename some as private - add pylint commands

- fix README.md error
- refactor several functions in app.py and rename some as private - add pylint commands
Miguel Barão
1 parent 8ce81519
Showing 10 changed files with 123 additions and 94 deletions Show diff stats
README.md
package.json
perguntations/app.py
perguntations/initdb.py
perguntations/main.py
perguntations/parser_markdown.py
perguntations/questions.py
perguntations/serve.py
perguntations/static/js/admin.js
perguntations/templates/test.html
@@ -39,7 +39,7 @@ This file is usually in `~/.config/pip/` in Linux and FreeBSD. In MacOS it&#39;s in
 Download and install:
 ```sh
-git clone https://git.xdi.uevora.pt/perguntations.git
+git clone https://git.xdi.uevora.pt/mjsb/perguntations.git
 cd perguntations
 npm install
 pip3 install .
@@ -225,7 +225,7 @@ Python packages can be upgraded independently of the rest using pip:
 ```sh
 pip list --outdated         # lists upgradable packages
-pip install -U something    # upgrade something 
+pip install -U something    # upgrade something
 ```
 To upgrade perguntations and javascript libraries do:
@@ -2,13 +2,13 @@
     "description": "Javascript libraries required to run the server",
     "email": "mjsb@uevora.pt",
     "dependencies": {
-        "@fortawesome/fontawesome-free": "^5.13.0",
-        "bootstrap": "^4.4.1",
-        "codemirror": "^5.53.2",
+        "@fortawesome/fontawesome-free": "^5.15.1",
+        "bootstrap": "^4.5.3",
+        "codemirror": "^5.58.1",
         "datatables": "^1.10",
         "jquery": "^3.5.1",
-        "mathjax": "^3.0.5",
+        "mathjax": "^3.1.2",
         "popper.js": "^1.16.1",
-        "underscore": "^1.10"
+        "underscore": "^1.11.0"
     }
 }
@@ -89,23 +89,7 @@ class App():
         self.unfocus = set()    # set of students that have no browser focus
         self.area = dict()      # {uid: percent_area}
-        logger.info('Loading test configuration "%s".', conf["testfile"])
-        try:
-            testconf = load_yaml(conf['testfile'])
-        except Exception as exc:
-            logger.critical('Error loading test configuration YAML.')
-            raise AppException(exc)
-
-        testconf.update(conf)  # command line options override configuration
-
-        # start test factory
-        try:
-            self.testfactory = TestFactory(testconf)
-        except TestFactoryException as exc:
-            logger.critical(exc)
-            raise AppException('Failed to create test factory!')
-        else:
-            logger.info('No errors found. Test factory ready.')
+        self._make_test_factory(conf)
         # connect to database and check registered students
         dbfile = self.testfactory['database']
@@ -117,26 +101,16 @@ class App():
                 num = sess.query(Student).filter(Student.id != '0').count()
         except Exception:
             raise AppException(f'Database unusable {dbfile}.')
-        else:
-            logger.info('Database "%s" has %s students.', dbfile, num)
+
+        logger.info('Database "%s" has %s students.', dbfile, num)
         # command line option --allow-all
         if conf['allow_all']:
-            logger.info('Allowing all students:')
-            for student in self.get_all_students():
-                self.allow_student(student[0])
+            self.allow_all_students()
         else:
             logger.info('Students not yet allowed to login.')
     # ------------------------------------------------------------------------
-    # FIXME unused???
-    # def exit(self):
-    #     if len(self.online) > 1:
-    #         online_students = ', '.join(self.online)
-    #         logger.warning(f'Students still online: {online_students}')
-    #     logger.critical('----------- !!! Server terminated !!! -----------')
-
-    # ------------------------------------------------------------------------
     async def login(self, uid, try_pw):
         '''login authentication'''
         if uid not in self.allowed and uid != '0':      # not allowed
@@ -175,6 +149,32 @@ class App():
         logger.info('"%s" logged out.', uid)
     # ------------------------------------------------------------------------
+    def _make_test_factory(self, conf):
+        '''
+        Setup a factory for the test
+        '''
+
+        # load configuration from yaml file
+        logger.info('Loading test configuration "%s".', conf["testfile"])
+        try:
+            testconf = load_yaml(conf['testfile'])
+        except Exception as exc:
+            logger.critical('Error loading test configuration YAML.')
+            raise AppException(exc)
+
+        testconf.update(conf)  # command line options override configuration
+
+        # start test factory
+        logger.info('Making test factory...')
+        try:
+            self.testfactory = TestFactory(testconf)
+        except TestFactoryException as exc:
+            logger.critical(exc)
+            raise AppException('Failed to create test factory!')
+
+        logger.info('Test factory ready. No errors found.')
+
+    # ------------------------------------------------------------------------
     async def generate_test(self, uid):
         '''generate a test for a given student'''
         if uid in self.online:
@@ -271,11 +271,11 @@ class App():
         '''handles browser events the occur during the test'''
         if cmd == 'focus':
             if value:
-                self.focus_student(uid)
+                self._focus_student(uid)
             else:
-                self.unfocus_student(uid)
+                self._unfocus_student(uid)
         elif cmd == 'size':
-            self.set_screen_area(uid, value)
+            self._set_screen_area(uid, value)
     # ------------------------------------------------------------------------
     # --- GETTERS
@@ -297,11 +297,11 @@ class App():
         cols = ['Aluno', 'Início'] + \
                [r for question in self.testfactory['questions']
-                  for r in question['ref']]
+                for r in question['ref']]
         tests = {}
-        for q in grades:
-            student, qref, qgrade = q[:2], q[2], q[3]
+        for question in grades:
+            student, qref, qgrade = question[:2], *question[2:]
             tests.setdefault(student, {})[qref] = qgrade
         rows = [{'Aluno': test[0], 'Início': test[1], **q}
@@ -351,13 +351,6 @@ class App():
                        .filter_by(id=test_id)\
                        .scalar()
-    def get_all_students(self):
-        '''get all students from database'''
-        with self.db_session() as sess:
-            return sess.query(Student.id, Student.name, Student.password)\
-                       .filter(Student.id != '0')\
-                       .order_by(Student.id)
-
     def get_student_grades_from_test(self, uid, testid):
         '''get grades of student for a given testid'''
         with self.db_session() as sess:
@@ -380,7 +373,15 @@ class App():
             'area': self.area.get(uid, None),
             'grades': self.get_student_grades_from_test(
                 uid, self.testfactory['ref'])
-            } for uid, name, pw in self.get_all_students()]
+            } for uid, name, pw in self._get_all_students()]
+
+    # --- private methods ----------------------------------------------------
+    def _get_all_students(self):
+        '''get all students from database'''
+        with self.db_session() as sess:
+            return sess.query(Student.id, Student.name, Student.password)\
+                       .filter(Student.id != '0')\
+                       .order_by(Student.id)
     # def get_allowed_students(self):
     #     # set of 'uid' allowed to login
@@ -409,30 +410,30 @@ class App():
     def allow_all_students(self):
         '''allow all students to login'''
-        logger.info('Allowing all students...')
-        self.allowed.update(s[0] for s in self.get_all_students())
+        self.allowed.update(s[0] for s in self._get_all_students())
+        logger.info('Allowed all students.')
     def deny_all_students(self):
         '''deny all students to login'''
         logger.info('Denying all students...')
         self.allowed.clear()
-    def focus_student(self, uid):
+    def _focus_student(self, uid):
         '''set student in focus state'''
         self.unfocus.discard(uid)
         logger.info('"%s" focus', uid)
-    def unfocus_student(self, uid):
+    def _unfocus_student(self, uid):
         '''set student in unfocus state'''
         self.unfocus.add(uid)
         logger.info('"%s" unfocus', uid)
-    def set_screen_area(self, uid, sizes):
+    def _set_screen_area(self, uid, sizes):
         '''set current browser area as detected in resize event'''
         scr_y, scr_x, win_y, win_x = sizes
         area = win_x * win_y / (scr_x * scr_y) * 100
         self.area[uid] = area
-        logger.info('"%s": area=%g%%, window=%dx%d, screen=%dx%d',
+        logger.info('"%s" area=%g%%, window=%dx%d, screen=%dx%d',
                     uid, area, win_x, win_y, scr_x, scr_y)
     async def update_student_password(self, uid, password=''):
 #!/usr/bin/env python3
 '''
-Commandline utilizty to initialize and update student database
+Commandline utility to initialize and update student database
 '''
 # base
@@ -15,10 +15,10 @@ import sys
 # from typing import Any, Dict
 # this project
-from .app import App, AppException
-from .serve import run_webserver
-from .tools import load_yaml
-from . import APP_NAME, APP_VERSION
+from perguntations.app import App, AppException
+from perguntations.serve import run_webserver
+from perguntations.tools import load_yaml
+from perguntations import APP_NAME, APP_VERSION
 # ----------------------------------------------------------------------------
@@ -123,9 +123,8 @@ def main():
         'review':   args.review,
         }
-    # testapp = App(config)
     try:
-        testapp = App(config)
+        app = App(config)
     except AppException:
         logging.critical('Failed to start application.')
         sys.exit(-1)
@@ -145,8 +144,7 @@ def main():
         sys.exit(-1)
     # --- run webserver ----------------------------------------------------
-    run_webserver(app=testapp, ssl_opt=ssl_opt, port=args.port,
-                  debug=args.debug)
+    run_webserver(app=app, ssl_opt=ssl_opt, port=args.port, debug=args.debug)
 # ----------------------------------------------------------------------------
+
 '''
 Parse markdown and generate HTML
 Includes support for LaTeX formulas
@@ -25,12 +26,19 @@ logger = logging.getLogger(__name__)
 # Block math: $$x$$ or \begin{equation}x\end{equation}
 # -------------------------------------------------------------------------
 class MathBlockGrammar(mistune.BlockGrammar):
+    '''
+    match block math $$x$$ and math environments begin{} end{}
+    '''
+    # pylint: disable=too-few-public-methods
     block_math = re.compile(r"^\$\$(.*?)\$\$", re.DOTALL)
     latex_environment = re.compile(r"^\\begin\{([a-z]*\*?)\}(.*?)\\end\{\1\}",
                                    re.DOTALL)
 class MathBlockLexer(mistune.BlockLexer):
+    '''
+    parser for block math and latex environment
+    '''
     default_rules = ['block_math', 'latex_environment'] \
                     + mistune.BlockLexer.default_rules
@@ -56,12 +64,19 @@ class MathBlockLexer(mistune.BlockLexer):
 class MathInlineGrammar(mistune.InlineGrammar):
+    '''
+    match inline math $x$, block math $$x$$ and text
+    '''
+    # pylint: disable=too-few-public-methods
     math = re.compile(r"^\$(.+?)\$", re.DOTALL)
     block_math = re.compile(r"^\$\$(.+?)\$\$", re.DOTALL)
     text = re.compile(r'^[\s\S]+?(?=[\\<!\[_*`~$]|https?://| {2,}\n|$)')
 class MathInlineLexer(mistune.InlineLexer):
+    '''
+    render output math
+    '''
     default_rules = ['block_math', 'math'] + mistune.InlineLexer.default_rules
     def __init__(self, renderer, rules=None, **kwargs):
@@ -70,13 +85,18 @@ class MathInlineLexer(mistune.InlineLexer):
         super().__init__(renderer, rules, **kwargs)
     def output_math(self, math):
+        '''render inline math'''
         return self.renderer.inline_math(math.group(1))
     def output_block_math(self, math):
+        '''render block math'''
         return self.renderer.block_math(math.group(1))
 class MarkdownWithMath(mistune.Markdown):
+    '''
+    render ouput latex
+    '''
     def __init__(self, renderer, **kwargs):
         if 'inline' not in kwargs:
             kwargs['inline'] = MathInlineLexer
@@ -85,19 +105,25 @@ class MarkdownWithMath(mistune.Markdown):
         super().__init__(renderer, **kwargs)
     def output_block_math(self):
+        '''render block math'''
         return self.renderer.block_math(self.token['text'])
     def output_latex_environment(self):
+        '''render latex environment'''
         return self.renderer.latex_environment(self.token['name'],
                                                self.token['text'])
 class HighlightRenderer(mistune.Renderer):
+    '''
+    images, tables, block code
+    '''
     def __init__(self, qref='.'):
         super().__init__(escape=True)
         self.qref = qref
     def block_code(self, code, lang='text'):
+        '''render code block with syntax highlight'''
         try:
             lexer = get_lexer_by_name(lang, stripall=False)
         except Exception:
@@ -107,6 +133,7 @@ class HighlightRenderer(mistune.Renderer):
         return highlight(code, lexer, formatter)
     def table(self, header, body):
+        '''render table'''
         return '<table class="table table-sm"><thead class="thead-light">' \
                + header + '</thead><tbody>' + body + '</tbody></table>'
@@ -141,14 +168,17 @@ class HighlightRenderer(mistune.Renderer):
     # Pass math through unaltered - mathjax does the rendering in the browser
     def block_math(self, text):
         '''bypass block math'''
+        # pylint: disable=no-self-use
         return fr'$$ {text} $$'
     def latex_environment(self, name, text):
         '''bypass latex environment'''
+        # pylint: disable=no-self-use
         return fr'\begin{{{name}}} {text} \end{{{name}}}'
     def inline_math(self, text):
         '''bypass inline math'''
+        # pylint: disable=no-self-use
         return fr'$$$ {text} $$$'
@@ -13,7 +13,7 @@ from typing import Any, Dict, NewType
 import uuid
 # this project
-from .tools import run_script, run_script_async
+from perguntations.tools import run_script, run_script_async
 # setup logger for this module
 logger = logging.getLogger(__name__)
 #!/usr/bin/env python3
 '''
-Handles the web and html part of the application interface.
-The tornadoweb framework is used.
+Handles the web, http & html part of the application interface.
+Uses the tornadoweb framework.
 '''
@@ -40,8 +40,8 @@ class WebApplication(tornado.web.Application):
             (r'/review', ReviewHandler),
             (r'/admin', AdminHandler),
             (r'/file', FileHandler),
-            # (r'/root',      MainHandler), # FIXME
-            # (r'/ws',            AdminSocketHandler),
+            # (r'/root', MainHandler),
+            # (r'/ws', AdminSocketHandler),
             (r'/adminwebservice', AdminWebservice),
             (r'/studentwebservice', StudentWebservice),
             (r'/', RootHandler),
@@ -66,7 +66,7 @@ def admin_only(func):
     Decorator used to restrict access to the administrator.
     Example:
-        @admin_only()
+        @admin_only
         def get(self): ...
     '''
     @functools.wraps(func)
@@ -78,6 +78,7 @@ def admin_only(func):
 # ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class BaseHandler(tornado.web.RequestHandler):
     '''
     Handlers should inherit this one instead of tornado.web.RequestHandler.
@@ -87,7 +88,7 @@ class BaseHandler(tornado.web.RequestHandler):
     @property
     def testapp(self):
-        '''simplifies access to the application'''
+        '''simplifies access to the application a little bit'''
         return self.application.testapp
     def get_current_user(self):
@@ -158,6 +159,8 @@ class BaseHandler(tornado.web.RequestHandler):
 #         AdminSocketHandler.update_cache(chat)  # store msgs
 #         AdminSocketHandler.send_updates(chat)  # send to clients
+# ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class StudentWebservice(BaseHandler):
     '''
     Receive ajax from students in the test in response from focus, unfocus and
@@ -174,6 +177,7 @@ class StudentWebservice(BaseHandler):
 # ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class AdminWebservice(BaseHandler):
     '''
     Receive ajax requests from admin
@@ -202,6 +206,7 @@ class AdminWebservice(BaseHandler):
 # ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class AdminHandler(BaseHandler):
     '''Handle /admin'''
@@ -260,6 +265,7 @@ class AdminHandler(BaseHandler):
 # ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class LoginHandler(BaseHandler):
     '''Handle /login'''
@@ -282,6 +288,7 @@ class LoginHandler(BaseHandler):
 # ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class LogoutHandler(BaseHandler):
     '''Handle /logout'''
@@ -296,6 +303,7 @@ class LogoutHandler(BaseHandler):
 # ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class RootHandler(BaseHandler):
     '''
     Handles / to redirect students and admin to /test and /admin, resp.
@@ -315,6 +323,7 @@ class RootHandler(BaseHandler):
 # ----------------------------------------------------------------------------
 # Serves files from the /public subdir of the topics.
 # ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class FileHandler(BaseHandler):
     '''
     Handles static files from questions like images, etc.
@@ -366,6 +375,7 @@ class FileHandler(BaseHandler):
 # ----------------------------------------------------------------------------
 # Test shown to students
 # ----------------------------------------------------------------------------
+# pylint: disable=abstract-method
 class TestHandler(BaseHandler):
     '''
     Generates test to student.
@@ -438,20 +448,8 @@ class TestHandler(BaseHandler):
         self.render('grade.html', t=test, allgrades=allgrades)
-# ----------------------------------------------------------------------------
-# FIXME  should be a post in the test with command giveup instead of correct...
-# class GiveupHandler(BaseHandler):
-#     @tornado.web.authenticated
-#     def get(self):
-#         uid = self.current_user
-#         t = self.testapp.giveup_test(uid)
-#         self.testapp.logout(uid)
-
-#         # --- Show result to student
-#         self.render('grade.html', t=t, allgrades=self.testapp.get_student_grades_from_all_tests(uid))
-
-
 # --- REVIEW -----------------------------------------------------------------
+# pylint: disable=abstract-method
 class ReviewHandler(BaseHandler):
     '''
     Show test for review
@@ -488,18 +486,20 @@ class ReviewHandler(BaseHandler):
             with open(path.expanduser(fname)) as jsonfile:
                 test = json.load(jsonfile)
         except OSError:
-            logging.error('Cannot open "%s" for review.', fname)
-            raise tornado.web.HTTPError(404)  # Not Found
+            msg = f'Cannot open "{fname}" for review.'
+            logging.error(msg)
+            raise tornado.web.HTTPError(status_code=404, reason=msg) from None
         except json.JSONDecodeError as exc:
-            logging.error('JSON error in "%s": %s', fname, exc)
-            raise tornado.web.HTTPError(404)  # Not Found
+            msg = f'JSON error in "{fname}": {exc}'
+            logging.error(msg)
+            raise tornado.web.HTTPError(status_code=404, reason=msg)
         self.render('review.html', t=test, md=md_to_html,
-                templ=self._templates)
+                    templ=self._templates)
 # ----------------------------------------------------------------------------
-def signal_handler(sig, frame):
+def signal_handler(*_):
     '''
     Catches Ctrl-C and stops webserver
     '''
@@ -119,7 +119,7 @@ $(document).ready(function() {
                         var checked = d['allowed'] ? 'checked' : '';
                         var password_defined = d['password_defined'] ? ' <span class="badge badge-secondary"><i class="fa fa-key" aria-hidden="true"></i></span>' : '';
                         var hora_inicio = d['start_time'] ? ' <span class="badge badge-success"><i class="fas fa-hourglass-start"></i> ' + d['start_time'].slice(11,16) + '</span>': '';
-                        var unfocus = d['unfocus']? ' <span class="badge badge-danger">unfocus</span>' : '';
+                        var unfocus = d['unfocus'] ? ' <span class="badge badge-danger">unfocus</span>' : '';
                         var area = '';
                         if (d['start_time'] ) {
                             if (d['area'] > 75)
@@ -44,7 +44,7 @@
 <!-- ===================================================================== -->
 <body>
 <!-- ===================================================================== -->
-<div class="progress fixed-top" style="height: 60px; border-radius: 0px;">
+<div class="progress fixed-top" style="height: 61px; border-radius: 0px;">
   <div class="progress-bar bg-secondary" role="progressbar" style="width: 100%" aria-valuenow="100" aria-valuemin="0" aria-valuemax="100"></div>
 </div>
	@@ -39,7 +39,7 @@ This file is usually in `~/.config/pip/` in Linux and FreeBSD. In MacOS it's in		@@ -39,7 +39,7 @@ This file is usually in `~/.config/pip/` in Linux and FreeBSD. In MacOS it's in
39	Download and install:	39	Download and install:
40		40
41	```sh	41	```sh
42	-git clone https://git.xdi.uevora.pt/perguntations.git	42	+git clone https://git.xdi.uevora.pt/mjsb/perguntations.git
43	cd perguntations	43	cd perguntations
44	npm install	44	npm install
45	pip3 install .	45	pip3 install .
	@@ -225,7 +225,7 @@ Python packages can be upgraded independently of the rest using pip:		@@ -225,7 +225,7 @@ Python packages can be upgraded independently of the rest using pip:
225		225
226	```sh	226	```sh
227	pip list --outdated # lists upgradable packages	227	pip list --outdated # lists upgradable packages
228	-pip install -U something # upgrade something	228	+pip install -U something # upgrade something
229	```	229	```
230		230
231	To upgrade perguntations and javascript libraries do:	231	To upgrade perguntations and javascript libraries do:
1	#!/usr/bin/env python3	1	#!/usr/bin/env python3
2		2
3	'''	3	'''
4	-Commandline utilizty to initialize and update student database	4	+Commandline utility to initialize and update student database
5	'''	5	'''
6		6
7	# base	7	# base
	@@ -13,7 +13,7 @@ from typing import Any, Dict, NewType		@@ -13,7 +13,7 @@ from typing import Any, Dict, NewType
13	import uuid	13	import uuid
14		14
15	# this project	15	# this project
16	-from .tools import run_script, run_script_async	16	+from perguntations.tools import run_script, run_script_async
17		17
18	# setup logger for this module	18	# setup logger for this module
19	logger = logging.getLogger(__name__)	19	logger = logging.getLogger(__name__)
	@@ -119,7 +119,7 @@ $(document).ready(function() {		@@ -119,7 +119,7 @@ $(document).ready(function() {
119	var checked = d['allowed'] ? 'checked' : '';	119	var checked = d['allowed'] ? 'checked' : '';
120	var password_defined = d['password_defined'] ? ' <span class="badge badge-secondary"><i class="fa fa-key" aria-hidden="true"></i></span>' : '';	120	var password_defined = d['password_defined'] ? ' <span class="badge badge-secondary"><i class="fa fa-key" aria-hidden="true"></i></span>' : '';
121	var hora_inicio = d['start_time'] ? ' <span class="badge badge-success"><i class="fas fa-hourglass-start"></i> ' + d['start_time'].slice(11,16) + '</span>': '';	121	var hora_inicio = d['start_time'] ? ' <span class="badge badge-success"><i class="fas fa-hourglass-start"></i> ' + d['start_time'].slice(11,16) + '</span>': '';
122	- var unfocus = d['unfocus']? ' <span class="badge badge-danger">unfocus</span>' : '';	122	+ var unfocus = d['unfocus'] ? ' <span class="badge badge-danger">unfocus</span>' : '';
123	var area = '';	123	var area = '';
124	if (d['start_time'] ) {	124	if (d['start_time'] ) {
125	if (d['area'] > 75)	125	if (d['area'] > 75)
	@@ -44,7 +44,7 @@		@@ -44,7 +44,7 @@
44	<!-- ===================================================================== -->	44	<!-- ===================================================================== -->
45	<body>	45	<body>
46	<!-- ===================================================================== -->	46	<!-- ===================================================================== -->
47	-<div class="progress fixed-top" style="height: 60px; border-radius: 0px;">	47	+<div class="progress fixed-top" style="height: 61px; border-radius: 0px;">
48	<div class="progress-bar bg-secondary" role="progressbar" style="width: 100%" aria-valuenow="100" aria-valuemin="0" aria-valuemax="100"></div>	48	<div class="progress-bar bg-secondary" role="progressbar" style="width: 100%" aria-valuenow="100" aria-valuemin="0" aria-valuemax="100"></div>
49	</div>	49	</div>
50		50