From 8c9a59d9c2d3bfe9d19673834b5a7103fad671e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 13 Apr 2025 23:10:43 +0200 Subject: [PATCH 1/3] Use "localized" sorting for keys of ini fields This way, the order of items in a given section is properly alphabetical and does not depend on the order in which the fields were added. --- TODO | 1 - grader/applications.py | 31 ++++++++++++++++++++++++++++++- grader/test_applications.py | 28 ++++++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/TODO b/TODO index b306082..bb1357c 100644 --- a/TODO +++ b/TODO @@ -1,6 +1,5 @@ - add tab-completion for label during motivation grading - 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 `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/applications.py b/grader/applications.py index 95ae1ec..13c0985 100644 --- a/grader/applications.py +++ b/grader/applications.py @@ -1,11 +1,13 @@ import collections import configparser +import contextlib import csv from fnmatch import fnmatch import functools import io import itertools import keyword +import locale import math import pprint import re @@ -15,6 +17,17 @@ from . import (person, vector, util) from .util import printff + +@contextlib.contextmanager +def override_locale(category, locale_string): + # Context manager to set locale temporarily. + # Strangely, it seems that there is no builtin that does that. + prev_locale_string = locale.getlocale(category) + locale.setlocale(category, locale_string) + yield + locale.setlocale(category, prev_locale_string) + + DEBUG_MAPPINGS = False # CSV-file: @@ -382,7 +395,23 @@ def save(self, filename=None): def save_to_file(self, file): # save our data to the INI file cp = configparser.ConfigParser(comment_prefixes='#', inline_comment_prefixes='#') - cp.read_dict(self.data) + + with override_locale(locale.LC_COLLATE, 'en_US.UTF-8'): + # The order of sections is kept stable. + # + # Within a section, we sort the fields alphabetically, + # with the en_US.UTF-8 collation, which puts accented + # letters in the expected place, usually right after the + # unaccented version. + sorted_data = { + name:{ + k:values[k] for k in sorted(values, + key=functools.cmp_to_key(locale.strcoll)) + } + for name, values in self.data.items() + } + + cp.read_dict(sorted_data) cp.write(file) name = getattr(file, 'name', '(tmp)') diff --git a/grader/test_applications.py b/grader/test_applications.py index 7831112..f47a426 100644 --- a/grader/test_applications.py +++ b/grader/test_applications.py @@ -1,4 +1,3 @@ -import io import pathlib import os import time @@ -57,6 +56,31 @@ def test_all_years(path): """ +ini_sorted = """\ +[extra] +key_num = 111.5 +key_str = value + +[motivation_score-zbyszek] +jędrzej marcin mirosławski piołun = 1 +person one = 1 +person three = 0 +person two = -1 +some son jr. = -1 + +[motivation_score-other] +person one = -1 +person two = 1 +some son jr. = 1 + +[labels] +jędrzej marcin mirosławski piołun = UTF-8, VEGAN +john doe = VEGAN, VIP +person one = PALEO + +""" + + def get_ini(tmp_path, *extra, ini_filename='ini1.ini'): input = tmp_path / ini_filename input.write_text(ini_string + '\n'.join(extra)) @@ -167,7 +191,7 @@ def test_applications_ini_save(tmp_path): out = tmp_path / 'ini1.copy' ini.save(out) - assert out.read_text() == ini_string + assert out.read_text() == ini_sorted # Replace an exisiting entry. # We use an int, but the type is converted to float internally From 7895d4e4e43ed8b18e3878e183859b87b7903108 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 14 Apr 2025 08:38:20 +0200 Subject: [PATCH 2/3] grader/applications: when saving, write text in one step Previously, if the generation of the ini text failed, we would end up with an empty or truncated file. This is not very likely to happen during normal use, but when developing the previous patch, we hit it many times. This file is very precious, so let's make an effort to avoid that. We now generate the contents first, and then write them using Path.write_text(). This is not atomic, but much less likely to fail halfway. --- grader/applications.py | 18 ++++++++++-------- grader/test_applications.py | 7 +++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/grader/applications.py b/grader/applications.py index 13c0985..2d8a188 100644 --- a/grader/applications.py +++ b/grader/applications.py @@ -389,10 +389,13 @@ def get_ratings(self, field): def save(self, filename=None): filename = filename or self.filename - with open(filename, 'wt') as file: - self.save_to_file(file) + text = self.to_string() - def save_to_file(self, file): + filename.write_text(text) + + printff(f'Saved changes to {filename}') + + def to_string(self): # save our data to the INI file cp = configparser.ConfigParser(comment_prefixes='#', inline_comment_prefixes='#') @@ -411,12 +414,11 @@ def save_to_file(self, file): for name, values in self.data.items() } - cp.read_dict(sorted_data) - cp.write(file) - - name = getattr(file, 'name', '(tmp)') - printff(f'Saved changes to {name}') + out = io.StringIO() + cp.read_dict(sorted_data) + cp.write(out) + return out.getvalue() def __setitem__(self, key, value): # allow to set items in the section of the INI using a dotted form, for ex: diff --git a/grader/test_applications.py b/grader/test_applications.py index f47a426..6334e2f 100644 --- a/grader/test_applications.py +++ b/grader/test_applications.py @@ -316,11 +316,10 @@ def test_applications_labels(app): assert app['Person One'].remove_label('PALEO') is True assert app['Person One'].labels == [] - out = io.StringIO() - app.ini.save_to_file(file=out) + text = app.ini.to_string() - assert 'john doe = VEGAN, VIP' in out.getvalue() - assert 'jędrzej marcin mirosławski piołun = UTF-8, VEGAN' in out.getvalue() + assert 'john doe = VEGAN, VIP' in text + assert 'jędrzej marcin mirosławski piołun = UTF-8, VEGAN' in text def test_applications_all_labels(app): assert app.all_labels() == {'PALEO', 'UTF-8', 'VEGAN'} From 9d0332186f092ce398d93a64413274f1aa37e8b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 14 Apr 2025 09:18:18 +0200 Subject: [PATCH 3/3] grader/applications: keep order or _rating sections --- grader/applications.py | 9 +++++++-- grader/test_applications.py | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/grader/applications.py b/grader/applications.py index 2d8a188..f499cb2 100644 --- a/grader/applications.py +++ b/grader/applications.py @@ -406,10 +406,15 @@ def to_string(self): # with the en_US.UTF-8 collation, which puts accented # letters in the expected place, usually right after the # unaccented version. + # + # As an exception, the [*_rating] sections are not sorted, + # so that the items remain from "lowest" to "highest". sorted_data = { name:{ - k:values[k] for k in sorted(values, - key=functools.cmp_to_key(locale.strcoll)) + k:values[k] for k in sorted( + values, + key=(lambda x:0) if name.endswith('_rating') else functools.cmp_to_key(locale.strcoll), + ) } for name, values in self.data.items() } diff --git a/grader/test_applications.py b/grader/test_applications.py index 6334e2f..f1aaa05 100644 --- a/grader/test_applications.py +++ b/grader/test_applications.py @@ -49,6 +49,11 @@ def test_all_years(path): person two = 1 some son jr. = 1 +[verify_rating_is_unsorted_rating] +novice = +10.0 +competent = 0.0 +expert = -10.0 + [labels] john doe = VEGAN, VIP jędrzej marcin mirosławski piołun = UTF-8, VEGAN @@ -73,6 +78,11 @@ def test_all_years(path): person two = 1 some son jr. = 1 +[verify_rating_is_unsorted_rating] +novice = 10.0 +competent = 0.0 +expert = -10.0 + [labels] jędrzej marcin mirosławski piołun = UTF-8, VEGAN john doe = VEGAN, VIP