From c700f09ea5de745363c5c38c1f5b1be30fa90834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 13 Apr 2025 18:11:02 +0200 Subject: [PATCH 1/3] Fix a few issues reported by pyflakes --- grader/applications.py | 5 ++--- grader/cmd_completer.py | 2 +- grader/grader.py | 29 ++++++----------------------- grader/person.py | 2 -- grader/test_person.py | 12 +++++------- grader/vector.py | 3 --- 6 files changed, 14 insertions(+), 39 deletions(-) diff --git a/grader/applications.py b/grader/applications.py index 513d510..95ae1ec 100644 --- a/grader/applications.py +++ b/grader/applications.py @@ -7,7 +7,6 @@ import itertools import keyword import math -import os import pprint import re import token @@ -238,7 +237,7 @@ def load_applications_csv(file, field_name_overrides={}, relaxed=False, ini=None try: yield person.Person.from_row(fields, entry, relaxed=relaxed, ini=ini) - except Exception as exp: + except Exception: print(f'Exception raised on entry {count}:', entry) print('Detected fields:\n', fields) raise @@ -267,7 +266,7 @@ def __init__(self, file): try: file = open(file) print(f"loading '{self.filename}'") - except FileNotFoundError as e: + except FileNotFoundError: # if the file doesn't exist yet, we'll create it when writing #print(f'warning: {e}') file = None diff --git a/grader/cmd_completer.py b/grader/cmd_completer.py index 80a7adc..62c2be9 100644 --- a/grader/cmd_completer.py +++ b/grader/cmd_completer.py @@ -174,7 +174,7 @@ def do_loadpy(self, arg): with contextlib.redirect_stdout(sys.stderr): try: exec(fh.read(), self.__dict__) - except Exception as e: + except Exception: traceback.print_exc(file=sys.stdout) def do_shell(self, arg): diff --git a/grader/grader.py b/grader/grader.py index 0157c6d..d804cdb 100755 --- a/grader/grader.py +++ b/grader/grader.py @@ -1,13 +1,11 @@ #!/usr/bin/env python3 import argparse import collections -import itertools import logging import numbers import operator import os import pathlib -import pprint import random import re import readline @@ -16,16 +14,14 @@ import traceback import numpy as np + try: import pandas except ImportError: pandas = None -from . import cmd_completer +from . import cmd_completer, vector, applications from .flags import flags as FLAGS -from . import vector - -from .applications import Applications from .util import ( list_of_equivs, @@ -181,14 +177,14 @@ def __init__(self, csv_file, identity=None, history_file=None): super().__init__(histfile=history_file) self.identity = identity - self.applications = Applications(csv_file=csv_file) + self.applications = applications.Applications(csv_file=csv_file) self.archive = [] for path in sorted(csv_file.parent.glob('*/applications.csv'), reverse=True): # years before 2012 are to be treated less strictly relaxed = any(f'{year}-' in str(path) for year in range(2009,2012)) - old = Applications(csv_file=path, relaxed=relaxed) + old = applications.Applications(csv_file=path, relaxed=relaxed) self.archive.append(old) for person in self.applications: @@ -281,7 +277,6 @@ def do_autolabel(self, args): Note: the last ranking is used to sort applications""" opts = self.autolabel_options.parse_args(args.split()) N = opts.N - applications = self.applications try: ranked = self.last_ranking except AttributeError: @@ -289,11 +284,11 @@ def do_autolabel(self, args): counter = 0 for person in ranked: if person.highlander: - applications.add_labels(person.fullname, ['INVITE']) + self.applications.add_labels(person.fullname, ['INVITE']) else: counter += 1 if counter <= N: - applications.add_labels(person.fullname, ['SHORTLIST']) + self.applications.add_labels(person.fullname, ['SHORTLIST']) else: return @@ -745,18 +740,6 @@ def _assign_rankings(self, use_labels=False): if ini.formula is None: raise ValueError('formula not set yet') - #if self.ranking_done: - # return - #else: - # self.ranking_done = True - categories = {name:ini.get_ratings(name) - for name in - ('programming', - 'open_source', - 'python', - 'vcs', - 'underrep')} - nan_to_value = lambda n: LABEL_VALUES['__nan__'] if np.isnan(n) else n ordered = sorted(self.applications.people, diff --git a/grader/person.py b/grader/person.py index 067fcf9..0954de4 100644 --- a/grader/person.py +++ b/grader/person.py @@ -2,10 +2,8 @@ import dataclasses import datetime -import functools import re import math -import numpy as np from . import (applications, vector) diff --git a/grader/test_person.py b/grader/test_person.py index 49cf218..0482b93 100644 --- a/grader/test_person.py +++ b/grader/test_person.py @@ -1,14 +1,12 @@ +import csv import math import time +import numpy as np from grader.person import (convert_bool, Person, FormulaProxy) from grader.applications import ApplicationsIni, Applications import pytest -import numpy as np -import math - -import csv from .test_applications import get_ini @@ -90,14 +88,14 @@ def test_person(): def test_person_invalid_born(): args = MARCIN | dict(born='1700') with pytest.raises(ValueError): - p = Person(**args) + Person(**args) @pytest.mark.parametrize('field', ['gender', 'programming', 'python', 'position']) def test_person_invalid_field(field): args = MARCIN | dict(((field, 'cat'),)) with pytest.raises(ValueError): - p = Person(**args) + Person(**args) def test_person_attr_not_set(): p = Person(**MARCIN) @@ -108,7 +106,7 @@ def test_person_attr_not_set(): def test_person_unknown_field(): args = MARCIN | dict(unkwown_field='123') with pytest.raises(TypeError): - p = Person(**args) + Person(**args) def test_person_applied(): args = MARCIN | dict(applied='Yes') diff --git a/grader/vector.py b/grader/vector.py index 91c9b80..9f32c2f 100644 --- a/grader/vector.py +++ b/grader/vector.py @@ -73,9 +73,6 @@ def __getslice__(self, i, j): def __add__(self, other): return vector(super(vector,self).__add__(other)) - def argsort(self): - return vector(key for key,val in sorted(zip(self, xrange(len(self))))) - def mean(self): valid = [arg for arg in self if arg is not None] if not valid: From 88d29d6c2f4f62a0f52011782da8ded0311fd618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 13 Apr 2025 18:35:29 +0200 Subject: [PATCH 2/3] grader: finish the _rating suffix removal in formats --- grader/grader.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/grader/grader.py b/grader/grader.py index d804cdb..e6aa46a 100755 --- a/grader/grader.py +++ b/grader/grader.py @@ -70,10 +70,10 @@ def format_have_applied(person, width=3): affiliation: {p.affiliation} position: {p.position}{position_other} appl.prev.: {have_applied} -programming: {p.programming}{programming_description} [{programming_rating}] -python: {p.python} [{python_rating}] -vcs: {p.vcs} [{vcs_rating}] -open source: {p.open_source}{open_source_description} [{open_source_rating}] +programming: {p.programming}{programming_description} [{programming}] +python: {p.python} [{python}] +vcs: {p.vcs} [{vcs}] +open source: {p.open_source}{open_source_description} [{open_source}] ''' DUMP_FMT = '''\ @@ -96,10 +96,10 @@ def format_have_applied(person, width=3): MOTIVATION_DUMP_FMT = '''\ appl.prev.: {have_applied} position: {p.position}{position_other} -programming: {p.programming}{programming_description} [{programming_rating}] -python: {p.python} [{python_rating}] -vcs: {p.vcs} [{vcs_rating}] -open source: {p.open_source}{open_source_description} [{open_source_rating}] +programming: {p.programming}{programming_description} [{programming}] +python: {p.python} [{python}] +vcs: {p.vcs} [{vcs}] +open source: {p.open_source}{open_source_description} [{open_source}] motivation: %(bold)s{motivation}%(default)s\ {labels_newline}''' % COLOR @@ -121,9 +121,9 @@ def format_have_applied(person, width=3): ' {p.fullname:{fullname_width}} {email:{email_width}}') _RANK_FMT_DETAILED = ('{: 4} {p.rank: 4} {labels:{labels_width}} {p.score:6.3f}' ' [{motivation_scores}] [appl: {have_applied}]' - ' [prog: {programming_rating}] [python: {python_rating}]' - ' [{gender:^{gender_width}}] [git: {vcs_rating}]' - ' [os: {open_source_rating}]' + ' [prog: {programming}] [python: {python}]' + ' [{gender:^{gender_width}}] [git: {vcs}]' + ' [os: {open_source}]' ' {p.fullname:{fullname_width}} {email:{email_width}}' ' {p.travel_grant}' ' {nationality:{nationality_width}} {affiliation:{affiliation_width}}' From b06dc8807055e946bebc7a1e7dde2e51a7f9e8c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 13 Apr 2025 18:29:43 +0200 Subject: [PATCH 3/3] grader: implement 'b' --- TODO | 1 - grader/grader.py | 44 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/TODO b/TODO index 5a74a5d..b306082 100644 --- a/TODO +++ b/TODO @@ -2,6 +2,5 @@ - add common labels to criteria.txt - tab-completion does not work out of the box in the Mac terminal: https://github.com/ASPP/grader/issues/26 - if two reviewers add the same label there is a git-conflict -- add a `b` key to the grade verb that allows to go back to re-grade the previous motivation letter - add a `o` key to the grade verb that allows to set overrides. Fields that can be overridden must be tab-auto-completed and possible values for the field must be also tab-auto-completed. - verify that we can dynamically change the formula and this has an effect on the ranking diff --git a/grader/grader.py b/grader/grader.py index e6aa46a..683b923 100755 --- a/grader/grader.py +++ b/grader/grader.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 import argparse import collections +import enum import logging import numbers import operator @@ -169,6 +170,14 @@ def equal(a, b): # use normal comparison otherwise return a == b + +class GradingMoves(enum.Enum): + BACK = enum.auto() + SKIP = enum.auto() + NEXT = enum.auto() + DONE = enum.auto() + + class Grader(cmd_completer.Cmd_Completer): prompt = COLOR['green']+'grader'+COLOR['yellow']+'>'+COLOR['default']+' ' set_completions = cmd_completer.Cmd_Completer.set_completions @@ -572,15 +581,27 @@ def do_grade(self, arg): printff('Press ^C or ^D to stop') random.shuffle(todo) - for num, person in enumerate(todo): - progress = '┃ {:.1%} done, {} left to go ┃'.format((num + done_already) / total, - len(todo) - num) + + pos = 0 + + while True: + if pos >= len(todo): + break + + progress = '┃ {:.1%} done, {} left to go ┃'.format((pos + done_already) / total, + len(todo) - pos) sep_up = '\n┏'+(len(progress)-2)*'━'+'┓\n' sep_down = '\n┗'+(len(progress)-2)*'━'+'┛\n' print(sep_up+progress+sep_down) print() - if not self._grade(person, opts.disagreement is not None): - break + + match self.grade_one_motivation(todo[pos], opts.disagreement is not None): + case GradingMoves.NEXT | GradingMoves.SKIP: + pos += 1 + case GradingMoves.BACK: + pos = max(0, pos - 1) + case GradingMoves.DONE: + break do_grade.completions = _complete_name @@ -643,7 +664,7 @@ def _set_grading(self, person, score): person.set_motivation_score(score, identity=self.identity) printff('Motivation score set to {}', score) - def _grade(self, person, disagreement): + def grade_one_motivation(self, person, disagreement): "This is the function that does the loop asking question +1/0/-1/…" if disagreement: scores = self._get_gradings(person) @@ -664,12 +685,12 @@ def is_valid_score(choice): valid_choice = None while valid_choice not in applications.SCORE_RANGE: - prompt = 'Your choice {}/s/d/l LABEL [{}]? '.format(applications.SCORE_RANGE, default) + prompt = 'Your choice {}/b/s/d/l LABEL [{}]? '.format(applications.SCORE_RANGE, default) try: choice = input(prompt) except EOFError: print() - return False + return GradingMoves.DONE if len(choice) <= 2: clen = readline.get_current_history_length() @@ -685,9 +706,12 @@ def is_valid_score(choice): valid_choice = 0 case ['-']: valid_choice = -1 + case ['b']: + printff('going back one') + return GradingMoves.BACK case ['s'] | []: printff('person skipped') - return True + return GradingMoves.SKIP case ['d']: printff('showing person on request') self._dumpone(person, format='long') @@ -715,7 +739,7 @@ def is_valid_score(choice): if not equal(valid_choice, default): self._set_grading(person, valid_choice) - return True + return GradingMoves.NEXT def _score_with_labels(self, p, use_labels=False): add_score = 0