Доработка кода qtype_writeregex

Issue #363 new
Камо Сперцян created an issue

Определить несовершенства кода и доработать его до работоспособности.

Comments (134)

  1. Камо Сперцян reporter

    Не получается установить плагины для moodle. Скачал архив preg из этого репозитория и извлек содержимое в папку moodle. После запуска moodle выдал мне список плагинов для установки. Я нажал "Обновить moodle", после чего наблюдаю абсолютно пустую страницу, в которой ничего не происходит. Попробовал уже несколько раз.

  2. Oleg Sychev repo owner

    Там в архиве ошибка есть, уже нашли, в блоке формальных языков. Обновлю. Можно взять все аналогичные плагины с сайта Moodle, там без ошибок. А состав плагинов вы видите по плагину и место куда их распаковывать тоже...

  3. Камо Сперцян reporter

    Действительно, с пакетом с сайта Moodle всё сработало.

  4. Камо Сперцян reporter

    Олег Александрович, хотел уточнить несколько вопросов:

    1) каким вообще образом тестировать свои изменения в файлах? Я их вношу, сохраняю, но ничего не меняется - я специально допустил синтаксическую ошибку в файле question.php, но на неё мудл не поругался.

    2) как мне тестировать способы проверки регэксов? Я видел выбор варианта при создании вопроса, но опять же не понимаю, в какой момент вызывается соответствующая функция из writeregex.

    3) почему тип вопроса writeregex существовал ещё до того, как я установил соответствующий плагин? Сначала его не было, потом я добавил preg - он появился, но writeregex не было в папках.

  5. Oleg Sychev repo owner

    1) Чтобы мудл поругался надо выполнить действие, касающееся question.php Попытку вопроса например. Редактирование и сохранение в БД проходит без него, а вот ответ на вопрос оценивается обязательно уже через него...

    2) При оценке ответа студента прежде всего. При хинтинге тоже должна, но это надо смотреть. Из способов пока явно реализован только один, возможно и вызывается только он же. Или остальные в заглушках... Вообще все это можно делать не только через интерфейс пользователя, но и через юнит-тесты т.к. question.php специально создавался так чтобы быть тестируемым без всякой БД и прочего...

    3) не знаю; может вы его раньше туда копировали, потом стерли - а мудл уже его засек в первый раз... это надо по делу смотреть...

  6. Камо Сперцян reporter

    Проблема: функция validate_test_strings ругается, что сумма 4-х тестовых строк 40+30+20+10 не равна 100.

    Суть ошибки: в функции validate_test_strings суммарная оценка по строкам храниться в переменной типа float и сравнивается с 1 ($sumgrade != 1). Из-за внутреннего устройства сравнения периодически даже при $sumgrade == 1 это условие вернет true.

    Предлагаемое решение: округление значения $sumgrade до двух знаков после запятой перед сравнением ($sumgrade = round($sumgrade, 2)).

  7. Камо Сперцян reporter

    Проблема: отсутствует нумерация вариантов ответа и тестовых строк.

    Предлагаемое решение: добавить нумерацию возле названия каждого варианта ответа или строки (Регулярное выражение 1, Тестовая строка 2...).

  8. Камо Сперцян reporter

    Проблема: количество вариантов правильных regex-ов слишком велико.

    Предлагаемое решение: сократить количество вариантов правильных regex-ов по умолчанию до одного.

  9. Камо Сперцян reporter

    Проблема: любое обращение к вопросу writeregex после создания вызывает ошибку чтения из базы данных.

    Предлагаемое решение пока отсутствует.

  10. Камо Сперцян reporter

    Ошибка обнаружена - производится запрос к таблице mdl_qtype_writeregex_options, в которой отсутствует запрашиваемое поле usecase. Как я понял - это поле отвечает за чувствительность к регистру. Верно ли я понимаю, что необходимо добавить в БД этот столбец? И если да, то как это сделать правильно?

    P.S. Прикрепляю лог ошибки со стеком вызовов.Ошибка с бд.PNG

  11. Oleg Sychev repo owner

    usecase это поле регистрочуствительности (в данном случае, должены ли регексы преподавателя и ученика совпадать с точностью до регистра букв или регистр может различаться (кстати, не мешало бы задуматься об этом при сравнении автоматов). Наследуется (в смысле extra_question_fields) еще от shortanswer, но в БД его надо создать отдельно.

    Пока можно прописать install.xml только, а у себя удалить и снова поставить - либо учиться писать upgrade.php (там все просто, я показывал где почитать Потаповой). Для редактирования install.xml настоятельно рекомендуется пользоваться встроенным в Moodle редактором, хотя это и неудобно - вручную вы скорее всего что-нибудь не так введете....

  12. Камо Сперцян reporter

    С usecase разобрался - просто скопировал аналогичное поле из install.xml у preg. Беда настигла после обновления бд - отсутствует ещё одно поле - engine (движок для проверки, как я понял). Буду разбираться, как оно должно быть представлено, и попробую добавить.

  13. Камо Сперцян reporter

    Действительно. Что-то я не заметил его сразу. notation есть.

  14. Камо Сперцян reporter

    Всё, наконец-то запускаются вопросы writeregex. Проблема с бд решена.

    Решение: в файле install.xml для модуля writeregex добавлено описание полей usecase и engine.

    Сейчас вытолкну изменения.

  15. Oleg Sychev repo owner

    В норме надо еще их в upgrade.php прописать, но раз пока только у вас установлено - можете этого не делать...

  16. Камо Сперцян reporter

    Хорошо. Я всё равно попробую поразбираться в update.php.

  17. Oleg Sychev repo owner

    Там шаблон кода можно сгенерировать через сам мудль (там же где редактируется install.xml - см в администрировании сайта раздел разработка, там XMLDB-editor

  18. Камо Сперцян reporter

    А в сгенерированном шаблоне необходимо что-то менять?

  19. Oleg Sychev repo owner

    Камо, не злоупотребляйте моим временем! Там сами увидите...

  20. Камо Сперцян reporter

    Ошибка: при создании вопроса при нажатии на кнопку "Добавить 1 вариант ответа" в блоке регулярных выражений их количество становится равным 6. Независимо от того. сколько их было изначально. При том добавляются ещё и тестовые строки. При попытке добавить тестовые строки результат аналогичный.

  21. Камо Сперцян reporter

    Олег Александрович, подскажите, пожалуйста, как внедрить изменения файлов со строковыми константами в moodle? Я изменил файл moodle\question\type\writeregex\lang\ru\qtype_writeregex.php, строковая константа из которого вызывается методом get_string($label, 'qtype_writeregex') из файла edit_writeregex_form.php, однако изменения в moodle не появляются. С чем это может быть связано?

  22. Oleg Sychev repo owner

    1) Про количество вариантов ответов - там в форме есть вызовы функций типа add_per_answer_fields или что-то такое; среди их параметров настраивается сколько добавлять.... 2) Мудл кеширует строки, так что надо очистить кеш. Там в администрировании сайта в разделе разработки посмотрите, он внизу... Ну и проверить английский вариант конечно - я например тестовый сайт прежде всего на английском держу...

  23. Камо Сперцян reporter

    1) Насчет количества вариантов ответа я просто обозначит проблему, в принципе в общих чертах я уже с этим механизмом ознакомился, но всерьез пока проблему не решал. 2) Я в обоих языках внес изменения. Попробую перекэшировать, спасибо.

  24. Камо Сперцян reporter

    Не очень понимаю наличие функции add_per_answer_fields - по-моему, она совершенно не нужна. Она вызывается дважды при генерации страницы - для создания блока регэксов и блока тестовых строк. При этом она всего лишь вызывает соответствующую функцию в зависимости от переданных параметров, ничего при этом не делая. Какой-то бесполезный диспетчер. Считаю, что лучше сразу при генерации вызывать нужную функцию.

  25. Oleg Sychev repo owner

    У вас есть Git. Открыв аннотацию файлов вы можете найти коммит, который эту функцию создал. И соответствующее issue - номер будет в коммит-сообщении скорее всего и всю историю работы над этим проектом меня и Тима. Я мог бы рассказать, но умение добыть эту информацию самому научит вас большему :)

    Чтобы видеть смысл этого проекта вы должны посмотреть на него в рамках всей группы вопросов Moodle, а не WriteRegex по отдельности. Подключить знания по ООП. Там был занятный рефакторинг в интересах целой группы классов.

    Единственное надо посмотреть действительно ли она имеет смысл при создании блока тестовых строк, потому что ее первоначальное назначение связано именно с блоками ответов - т.е. в нашем случае регексов... Тестовые строки ответами не являются...

  26. Камо Сперцян reporter

    1) Изменил количество регулярных выражений по умолчанию. Решение - передал параметром нужное количество повторений в функцию генерации блока из функции генерации страницы. 2) Исправил ошибку, из-за которой при нажатии на кнопку добавления регулярных выражений или тестовых строк их количество в обоих блоках становилось равным 8. Ошибка заключалась в том, что за это количество у обеих групп отвечал один и тот же буффер. Решение - завел отдельный буффер для блока тестовых строк. 3) Заменил вызов функции add_per_answer_fields у edit_writeregex_form.php на вызов непосредственно нужной функции, без лишнего посредничества, описанного комментарием выше. Саму функцию add_per_answer_fields удалил за ненадобностью.

  27. Камо Сперцян reporter

    Насчет аннотации файла:

    Функция add_per_answer_fields изначально была копипастом функции из edit_question_form с небольшими изменениями - коммит 5719 "Titles of edited form has been changed."

    Далее было сделано разделение на добавление регэксов и тестовых строк и функция приобрела вид такой, какой он есть сейчас - коммиты 5722 "Function which add regexp strings (answers) has been created." и 5723 "Function which add test strings has been created."

  28. Oleg Sychev repo owner

    Посмотрел на код, там правда существует ерунда; но решаться она должна несколько не так. Надо все таки писать методы типа "qtype_writeregex_edit_form::add_per_answer_fields" чтобы не ошибаться. Там да, предыдущий автор не понял функции и зачем-то принялся перегружать функцию базового класса, да еще странным образом.

    Чтобы все это наладить нормально вам надо обратить внимание на question_edit_form::add_per_answer_fields - о которой я думал когда писал предыдущий комментарий - и которая наследуется. Она должна взять на себя функции нынешнего add_regexp_strings, который по большей части должен исчезнуть. А вот код add_test_strings может либо перекочевать прямо в definition_inner, либо вызываться оттуда. Вообще, я бы переместил немного функции так, чтобы все что связано с определением формы - добавление элементов - было рядом с definition_inner, и прочие аналогичные функции тоже были рядом.

    Проверьте также, на что сейчас влияет наследование от shortanswer. У меня такое подозрение что практически ни на что, с тем же успехом можно наследоваться и от question_edit_form. Ну может валидация наличия одного ответа на 100% только, но это не повод так наследоваться - можно и что-то нежелательное унаследовать...

  29. Oleg Sychev repo owner

    Вы пишите методы без указания класса, поэтому я подумал на соответствующий метод родительского класса. Вам надо разобраться с ним, т.к. использовать его в нормальной форме таки следует.

  30. Камо Сперцян reporter

    Разница между qtype_writeregex_edit_form::add_regexp_strings и question_edit_form::add_per_answer_fields заключается в передаче заголовка блока и вызове метода, возвращающего набор элементов для одного повторения (get_per_answer_fields_regexp и get_per_answer_fields соответственно). Причем я полагаю, что и эту функцию в qtype_writeregex_edit_form можно переименовать просто в get_per_answer_fields, и тогда разница будет только в заголовках блоков - можно сделать либо аргументом функции, либо вызовом метода, который будет перегружен в qtype_writeregex_edit_form, и тогда можно вообще избавиться от функции qtype_writeregex_edit_form::add_regexp_strings и вызывать соответствующий метод родителя.

  31. Oleg Sychev repo owner

    А что там за заголовки блоков в стандартном варианте? Они вполне могут нас и устроить, типа Answer 1, Answer 2 и т.д.

  32. Камо Сперцян reporter

    Вы не совсем правильно поняли. Под блоком я понимаю все ответы - то есть заголовок блока - это строка, по нажатию которой открываются и закрываются ответы. Стандартное название стоит "Ответы", у writeregex - "Регулярные выражения". А названия самих вариантов ("Ответ 1", "Ответ 2" или "Регулярное выражение 1"...) формируются уже в методе get_per_answer_fields каждого класса. У writeregex этот метод назван get_per_answer_fields_regexp, потому что по аналогии создан get_per_answer_fields_strings, отвечающий за одно повторение тестовой строки.

    Если нас устраивает заголовок "Ответы" вместо "Регулярные выражения", то в принципе отпадает необходимость "заморачиваться" с названием блока ответов и остается только переименовать метод get_per_answer_fields_regexp в get_per_answer_fields.

  33. Oleg Sychev repo owner

    Да пусть будет "Ответы" - заодно и пользователям яснее будет что это именно ответы...

  34. Камо Сперцян reporter

    Хорошо, понял. Значит сделаю так, как мы описали выше.

  35. Камо Сперцян reporter

    Проблема: при редактировании созданных вопросов показывается 5 повторений ответов вместо нужного количества (например, одного). Проблема: при прохождении вопроса с одним верным регэксом при проверке ответа всегда выдается сообщение о неверном ответе, даже если ответ правильный. При том при правильном ответе баллы за вопрос начисляются. Возможно, эта проблема связана с предыдущей.

  36. Oleg Sychev repo owner

    Проблемы врядли связаны, хотя надо последить за тем не сохраняются ли в БД пустые (без текста регекса) "ответы".

    Насчет проверки надо смотреть потому что она может быть полу-заглушкой в любом случае. Прежде всего надо разобраться в том, что возвращаются grade_answer (или что-то типа того, в question.php) и вызываемая из нее best_fit_answer.

  37. Камо Сперцян reporter

    Посмотрел БД - ответы и тестовые строки сохраняются в таблицу ответов. Я создавал вопрос с одним ответом и четырьмя тестовыми строками, потому и получилось, что ответов 5. Снимок.PNG

  38. Oleg Sychev repo owner

    Значит надо смотреть код, который их различает - строки от регексов. При подсчете количества которое надо отобразить он работает неверно, принимая строки за тоже регексы. Хранение строк вместе с регексами в таблице ответов с различением по полю (его можно найти в коде) было сознательным решением, его менять не надо. Или answerformat или что-то подобное.

  39. Камо Сперцян reporter

    Насчет описанной выше проблемы с количеством ответов при редактировании вопроса. Скорее всего проблема в следующей строке

    $repeatsatstart = count($this->question->options->$answersoption);

    функции question_edit_form::add_per_answer_fields, вызываемой из наследника qtype_writeregex_edit_form. Эта строка не различает регэйксы и тестовые строки для вопроса writeregex. Я предлагаю 2 варианта решения:

    1) заменить эту строку в классе question_edit_form на вызов функции, возвращающей количество ответов по переданному $question; в самом классе question_edit_form эта функция будет возвращать результат указанной строки, а в классе qtype_writeregex_edit_form переопределить этот метод и возвращать именно количество регэксов.

    2) переопределить функцию add_per_answer_fields в классе qtype_writeregex_edit_form (получится копипаст, от которого мы недавно избавились).

  40. Oleg Sychev repo owner

    А что, обеспечить разные значения $answersoption для регексов и строк никак нельзя?

  41. Камо Сперцян reporter

    Честно говоря, я не очень понимаю вообще обращения к полю $answersoption - его нет как такового. Снимок.JPG

  42. Oleg Sychev repo owner

    Разберитесь что означает в PHP второй доллар в вашей строчке $this->question->options->$answersoption

    Обратите внимание, что после двух стрелок доллара не стоит, а после третьей стоит. Наверное есть разница?

  43. Oleg Sychev repo owner

    И да - лучше давайте строки в виде ссылок на код в битбакете, это возможно. Так удобнее видеть строку в контексте...

  44. Камо Сперцян reporter

    Разобрался. answeroptions содержит название поля, в данном случае answers. И всё равно, я не понимаю, как Вы хотите. чтобы я разделил из 4-х имеющихся в этом примере ответов 1 регэкс и 3 тестовые строки. Они все хранятся в одном массиве и отличаются только парочкой полей внутри своих значений.

  45. Oleg Sychev repo owner

    Вот по полям и разделять. Я кажется ссылку на код просил...

  46. Камо Сперцян reporter

    Это строка из файла edit_question_form.php, которого нет в наших репозиториях, он базовый для question.

    Вы сказали, что хранение регэксов и тестовых строк вместе было сознательным решением. Как в таком случае мне следует их разделить?

  47. Oleg Sychev repo owner

    Ну и подумать своей головой никак? Откуда берется значение $answeroptions? Его то вы можете менять из вопроса?

    Теперь $this->question->options - какая функция заполняет это поле? Она под вашим контролем? Она может разбить регексы и строки на два отдельных поля внутри этого объекта опций? (чтобы ответить на последний вопрос возможно придется прочитать реализации функций в дочерних классах, но это в любом случае пригодится).

  48. Камо Сперцян reporter

    Олег Александрович, на последней паре мы пришли к тому, что мне нужно переопределить метод question_type::initialise_question_instance в классе qtype_writeregex - добавить туда тестовые строки. Но в каком виде необходимо их туда добавить? Добавить их так же, как и ответы (кстати ответы инициализируются отдельно от остальной части методом question_type::initialise_question_answers)? Но для каждого ответа создаётся экземпляр класса question_answer. Как мне здесь правильно поступить?

    И ещё один вопрос. Я, помнится, спрашивал уже, но не помню точно ответ - как можно будет впоследствии протестировать то, что я напишу? Этой функцией получаются те самые вопросы, которые в тесте используются для прохождения, верно?

  49. Oleg Sychev repo owner

    Можно создать либо свой класс (уже в пространстве имен) qtype_writeregex\test_string (или testing_string - как они в коде там назывались?), либо использовать std_class - надо же хранить как минимум строку и оценку. В сам класс вопроса добавляем поле из массива таких объектов.

    Проверить можно тем, что при загрузке из БД эта функция создает класс объекта вопроса, используемый в дальнейшем при оценивании и т.д. Надо будет кстати посмотреть как в оценивании вызывается анализатор по тестовым строкам и передаются туда эти самые строки - вполне возможно, его надо будет поправить. После этого уже можно реально попробовать создать вопрос, ввести регекс и потестить на некоторых строках - пусть пока вся оценка по строкам идет...

    P.S. На следующей лекции готовятся докладывать Напалков, Пестун и Скориков. Л/р также проводится всей группой с 8-30, а вторая часть отводится под консультации по курсовому, на следующих двух лабораторных возвращаемся в обычный режим.

  50. Камо Сперцян reporter

    Я понял, значит буду делать по аналогии с ответами. Насчет загрузки из БД - при каких действиях она осуществляется, чтобы можно было отловить и проследить этот процесс?

    Про лекцию всем передам.

  51. Oleg Sychev repo owner

    Да при любых действиях с вопросом из интерфейса пользователя. Вот вы создали вопрос, а потом вызвали предпросмотр (лупа) например и с ним там работаете - это самое простое, чтобы quiz не создавать...

  52. Камо Сперцян reporter

    Олег Александрович, по структуре teststrings и answer совпадают. Так может использовать для инициализации тестовых строк тот же класс (question_answer), что и для ответов? Или продублировать и переименовать этот класс?

  53. Oleg Sychev repo owner

    Я бы наверное завел свой класс. question_answer мало того что сбивает с толку в итак не самом простом коде, так еще и не под нашим контролем и может измениться не так, как нам надо. А мы тоже можем захотеть что-то изменить в строках...

  54. Камо Сперцян reporter

    При вызове подсказки графа объяснения возникает ошибка.

    Ошибка возникает в методе:

    qtype_preg_regex_handler::execute_dot($dotscript, 'svg');

    по следующему условию:

    if (empty($CFG->pathtodot)) { throw new qtype_preg_pathtodot_empty(''); }

    С чем это может быть связано?

  55. Oleg Sychev repo owner

    С тем, что нужно поставить Graphviz и настроить в настройках сервера Moodle путь к утилите dot которую код использует. Погуглите что такое пакет graphviz и/или почитайте в инструкциях к Preg по regex constructor/authoring tools.

    Да и еще - посмотрите какую ошибку (их там более одной, есть еще когда путь непустой, а dot не работает вроде) выдаст Preg при попытке показать граф без dot - и пропишите прием этого исключения и выдачу соответствующей ошибки для WriteRegex. Обратите внимание на то, что в нашем случае ошибка должна выдаваться прямо при создании вопроса - если кто-то пытается включить подсказки дерева/графа не имея dot надо ему выдавать ошибку и не давать сохранить такой вопрос. Там есть функция, проверяющая наличие dot.

  56. Камо Сперцян reporter

    Собственно, при вызове подсказки синтаксического дерева ошибка та же.

  57. Oleg Sychev repo owner

    Разумеется, они оба пользуют dot. Разберитесь с graphviz, там несложно, или работайте пока с описанием...

  58. Камо Сперцян reporter

    Буду значит ставить себе graphviz.

    Я наткнулся только на эту ошибку, до других просто не доходит. Это самое первое условие.

  59. Камо Сперцян reporter

    Олег Александрович, почему даже при наличии ответа студента функция hint_available вызывается без аргументов (response == null всегда)? Как мне получить ответ студента?

  60. Oleg Sychev repo owner

    Там разные ситуации, зависит от поведения. Попробуйте grep'нуть вызовы - найдете и те которые без аргументов, и те что с аргументами. Если по коду не разберетесь в каких случаях делается вызов с указанием ответа студента - кидайте ссылку на найденный вызов, попробую объяснить.

  61. Камо Сперцян reporter

    Проверяю на подсказке по тестовым строкам. Метод hint_available вызывается при проверке ответа студента или использовании подсказки без аргументов вот здесь. При вызове подсказки по тестовым строкам метод hint_available вызывается с передачей response тут. Меня устроит такое поведение, когда я буду проверять доступность подсказки при её вызове? Я так понял, что это связано как раз с той багой, что Вы рассказывали, когда можно изменить ответ и сразу нажать подсказку.

  62. Камо Сперцян reporter

    И еще у меня такая проблема - я не знаю, как узнать, что ответ студента не может получить подсказку по тестовым строкам.

    Когда я создаю qtype_preg_regex_testing_tool, он в массив errormsg записывает ошибку, но массив приватный, поэтому получить к нему доступ из хинта я не могу. А вызов метода data_for_accepted_regex() вылетает на этой строчке, потому что matcher = null. При том, этот эксепшн не ловится блоком try catch, несмотря на то, что вызов метода идет внутри него.

    Как я могу решить эту проблему?

  63. Камо Сперцян reporter

    Похожая ситуация у Description hint - там есть метод get_errors, но он из общего массива ошибок возвращает только производные от qtype_preg_accepting_error и qtype_preg_modifier_error, тогда как при регэксе, например, "-[" возникает qype_preg_parse_error. Но функция get_errors вернет пустой массив.

    При этом при вызове самой подсказки эта ошибка демонстрируется пользователю. Как поступить? Снимок.PNG

  64. Oleg Sychev repo owner

    Много вопросов.

    1) В adaptive_hints я так понимаю можно было бы сформулировать поведение так по вашим ссылкам: она выдает кнопку если подсказка вообще разрешена (вызов без параметров), а потом при нажатии кнопки проверяет реальную возможность при данном ответе (вызов с параметрами), если нет - то не показывает хинт. Это вполне разумно, лучшей стратегии в том поведении не получится...

    2) В qtype_preg_regex_testing_tool я думаю единственное разумное решение - дописать публичные методы, аналогичные методам хендлера errors_exist() и get_error_messages(). Можно сделать их пробросом вызова к матчеру чтобы не дублировать код, организовав запоминание матчера в https://bitbucket.org/oasychev/moodle-plugins-preg/src/6e7bcaf940cb02e291676f94f7e669374a753560/question/type/preg/authoring_tools/preg_regex_testing_tool.php?at=default&fileviewer=file-view-default#preg_regex_testing_tool.php-73 вне ветки else а всегда, после развилки.

    3) Ситуация у Description hint (равно как и у Syntax tree) немного иная. Ошибка с точки зрения хинта - это то, что он не может отобразить. В данном случае описание вполне может отобразить ошибку, как и дерево, и это хорошо - студент увидит ее положение в структуре регекса, ему будет легче понять чем просто сообщение об ошибке.

  65. Камо Сперцян reporter

    1 и 3 понял.

    Не до конца понял про проброс метода к матчеру, но это я покопаю ещё. Вот что меня интересует - я могу добавлять методы в qtype_preg_regex_testing_tool в рамках writeregex? Это же часть preg,

  66. Oleg Sychev repo owner

    Добавлять можете если я прямо разрешил вам это сделать, не по собственной инициативе. "Проброс" это просто вызвать аналогичный метод матчера (который там агрегируется, а не наследуется) и вернуть результаты.

    Технически это один репозиторий, так что проблем не будет.

  67. Камо Сперцян reporter

    В конструкторе qtype_preg_regex_testing_tool matcher инициализируется только если регэкс не содержит ошибок, поэтому осуществить проброс я не могу. Мне возвращать ошибки из самого qtype_preg_regex_testing_tool или исправить конструктор, чтобы матчер задавался в любом случае?

    Вот код, где это происходит.

  68. Oleg Sychev repo owner

    Вы читали что я выше написал? "Можно сделать их пробросом вызова к матчеру чтобы не дублировать код, организовав запоминание матчера в https://bitbucket.org/oasychev/moodle-plugins-preg/src/6e7bcaf940cb02e291676f94f7e669374a753560/question/type/preg/authoring_tools/preg_regex_testing_tool.php?at=default&fileviewer=file-view-default#preg_regex_testing_tool.php-73 вне ветки else а всегда, после развилки."

  69. Камо Сперцян reporter

    Результаты PearReview.

    1. Хинты и анализаторы в автозагрузку. По файлу на класс. Массив анализаторов возвращать из questiontype функцией по названиям без "_analyzer", при создании вопроса получать и выдавать список отсюда, также заменить использование в question.php. Из 5929 Попробовать имена percentage-й использовать в массиве.

    2. По возможности использовать одинарные кавычки.

    3. 5915 protected - зачем? Сделать это отдельным коммитом.

    4. 5921 переименовать $key и &a.

    5. 5924 соединить с коммитом, в котором допущена опечатка.

    6. 5927 item заменить на string, matcherstd - studentsmatcher, matchert - teachersmatcher.

    7. 5929 Добавить комментарий к поиску начального ответа.

    8. 5932 В рендерере создание тегов для таблицы сделать через html_writer и htmlspetialchars (см. Preg/renderer).

    9. 5934 Заголовки оборачивать в тег через renderer. В хинтах вызывать этот метод.

    10. 5936 Строковые константы переделать, получать с передачей аргумента - названия инструмента. Не забыть про заглавную букву.

    11. 5943 Посмотреть, для чего нужны строки explgraphhinttype - что за Count explanation?

    12. 5947 В коммит сообщении указать про замену html на json.

    13. 5951 Заменить get_errors на errors_exists, добавить get_error_messages (см. preg_regex_handler).

    14. Подумать над выделением общего класса для hint-а.

    15. Режимы mode заменить на константы в базовом классе, когда он будет создан.

  70. Камо Сперцян reporter

    Исправлены все недочёты, кроме хинтов. Осталось их сделать автозагружаемыми и выделить базовый класс, а также зависимая от этого 15-я задача из списка выше.

  71. Камо Сперцян reporter

    Олег Александрович, сейчас занимаюсь автозагрузкой хинтов, стоит ли для них сделать отдельные папку и namespace?

  72. Камо Сперцян reporter

    Всё, наконец-то закончил.. Вроде все пункты выполнил, кроме кавычек (2). Буду их править параллельно с codechecker-ом.

    Запушил всё.

    P.S. Базовый класс для хинтов встал более чем превосходно. Кода стало в 4 раза меньше.

  73. Камо Сперцян reporter

    Я что-то нехорошее натворил, у меня наплодились разные головы... Постараюсь исправить.

  74. Oleg Sychev repo owner

    В принципе можно сделать отдельный namespace для хинтов, но в случае writeregex не думаю что в этом есть смысл - там не так уж много кода вообще в вопросе будет. Весь основной расчетный код все равно в preg. Вот там классов завались и отдельный namespace много для чего будет...

  75. Oleg Sychev repo owner

    По веткам вижу, теперь Пенского научите...

    По коммиту 8d4b0bb - я вообще-то имел ввиду чуть больше. Как минимум для трех видов подсказок (кроме тестовых строк) строки с названиями можно взять из имеющихся у вас. $string['syntaxtreehinttype'] = 'Syntax tree' почему бы от этой строки не плясать подставляя ее как часть $a? Не вижу смысла их дублировать. В этом случае можно использовать единую строку типа '{$a->type} {$a->mode}' - если убрать на кнопке скобки (ведь смысл фразы они не меняют?) а на заголовке двоеточие (раз он уже выделен как заголовок, может быть и без двоеточия). Единую строку таки надо, вдруг в каких-то языках они в другом порядке должны идти? Только на кнопке программным путем первую букву маленькой сделать.

    Надо стараться не дублировать строки по возможности, а использоваться имеющиеся.

  76. Oleg Sychev repo owner

    В коммите c6c918a все хорошо, но посмотрите на строковые файлы - там с алфавитом какие-от проблемы, потому что xxx_title находится выше чем просто xxx и xxx_help. Поскольку не все такие группы строк затрагиваются этим коммитом, лучше сделать отдельный коммит исправляющий этот порядок строк.

  77. Oleg Sychev repo owner

    42be14d - опечатка в коммит сообщении, поправьте уж.

    ee06947 - посмотрите на редактируемые строки в question.php. Такое впечатление, что там назревает тоже решение, похожее на стандартизацию имен в analyzers. Там например 2 одинаковых if...else которые вполне можно заменить без условий. Можно вполне сделать и available_hint_types в типе вопроса. Только тут есть проблема составления из двух слов (где-то нужны подчеркивания, где-то нет). Один вариант ее решения в массиве возвращаемом available_hint_types сделать ключом например без подчеркивания, а значением - с подчеркиванием внутреннее имя. Другой вариант - сделать ключ с подчеркиванием и подобрать строковую функцию для его удаления в случаях, когда не нужно.

  78. Oleg Sychev repo owner

    72d6e38 - вы немного поскромничали с рефакторингом, функции render_hint_for_answer и render_hint_for_both_students_and_teachers_answers практически идентичны в 3х из 4х видах подсказок, что мешает вам поместить этот вариант в базовый класс, а перегрузить только для тестовых строк где функции меняются? Это будет удобнее копи-паста.

    3e64cbd - вы уверены, что извели все числа? Навскиду я вижу что не исправлялись (строки из предыдущего коммита, не затронутые в этом) https://bitbucket.org/SC_pRo_ION/moodle-plugins-preg-writeregex/commits/72d6e38d536637e934f97db2457cc49af65f1fba#Lquestion/type/writeregex/classes/hint.phpT104 и https://bitbucket.org/SC_pRo_ION/moodle-plugins-preg-writeregex/commits/72d6e38d536637e934f97db2457cc49af65f1fba#Lquestion/type/writeregex/classes/hint.phpT52

  79. Oleg Sychev repo owner

    P.S. К комментарию по PeerReview скиньте пожалуйста сюда табличку соответствия номер коммита-его хеш для упомянутых там коммитов, т.к. на сайте и у меня номера от ваших коммитов отличаются.

  80. Камо Сперцян reporter

    Вот соответствие коммитов по пунктам, для которых это нужно:

    1. Хинты и анализаторы в автозагрузку. По файлу на класс. Массив анализаторов возвращать из questiontype функцией по названиям без "_analyzer", при создании вопроса получать и выдавать список отсюда, также заменить использование в question.php. Из 5929 Попробовать имена percentage-й использовать в массиве.

    2. (371e8a0) 5921 переименовать $key и &a.

    3. (соединен с 714a99c) 5924 соединить с коммитом, в котором допущена опечатка.

    4. (67d5ca9) 5927 item заменить на string, matcherstd - studentsmatcher, matchert - teachersmatcher.

    5. (d11c9db) 5929 Добавить комментарий к поиску начального ответа.

    6. (07fbd91 -> 1368e6f) 5932 В рендерере создание тегов для таблицы сделать через html_writer и htmlspetialchars (см. Preg/renderer).

    7. (1a7e25b -> 37d0eb2, a74e119) 5934 Заголовки оборачивать в тег через renderer. В хинтах вызывать этот метод.

    8. (-> 8d4b0bb) 5936 Строковые константы переделать, получать с передачей аргумента - названия инструмента. Не забыть про заглавную букву.

    9. (23d69ff) 5947 В коммит сообщении указать про замену html на json.

    10. (85f4d80) 5951 Заменить get_errors на errors_exists, добавить get_error_messages (см. preg_regex_handler).

    По поводу моих недочётов:

    Насчёт 8d4b0bb - понял, исправлю.

    Насчёт c6c918a - мне полностью во всех языковых файлах расположить по алфавитному порядку? Или только в блоках, объединенных общим смыслом?

    Насчёт 42be14d - исправою.

    Насчёт ee06947 - посмотрю, попробую решить.

    Насчёт 72d6e38 - мне показалась такая реализация более корректной с точки зрения понимая. В принципе выделить в базовый класс эти методы реально, но в Descritpion_hint их реализация тоже немного отличается. потому что там используется html, а не json. И возвращаемое значение в обоих методах отлично от возвращаемых значений подсказок с графами. syntax_tree_hint и explanation_graph_hint действительно дублируют. Постараюсь максимально выделить в базовый класс.

    Насчёт 3e64cbd - я опирался на то, что строковые константы режимов используются для отображения. В двух местах действительно упустил численное представление. Исправлю и со строковым представлением постараюсь тоже сделать.

  81. Oleg Sychev repo owner

    По 72d6e38 (похоже уже бывшему) - посмотрите, если использовать json там одинаково не получится? Если не получится, можно вообще-то доработать generate_html в дереве и графе потому что его толком нельзя было протестить без вашего кода, больше он никому был не нужен. Чтобы лишних тегов не выдавал.

    3e64cbd - ну да, для mode - режима работы (студента/препода/обоих) - ну так в тех двух случаях на которые я дал ссылки она для этого и применяется.

    c6c918a - ну конкретно в этом коммите его строки нормально сделать. А вообще там в языковом файле полный бардак и порядок там стоит навести, иначе его поддерживать будет сложно. Это уже отдельным коммитом.

    1) разобраться что означает префикс wre - либо его всюду где надо сделать (и комментарий перед этой секцией, что она такое), либо убрать.

    2) Либо расположить весь файл по алфавиту, либо разбить на небольшое количество откоментированных секций (например форма редактирования вопроса, интерфейс вопроса в тесте и т.д.) а внутри секций уже по алфавиту

    3) Посмотреть неиспользуемые (через grep), а также дублирующиеся строки, удалить (для дублирующихся поправить код). Дублирующиеся также надо смотреть по мудлу, например строки для регистрочуствительности есть уже в qtype_shortanswer, можно прописать в version.php зависимость от него (все равно preg от него зависит) и использовать строки оттуда, не добавляя их свой файл (так вероятность перевода выше). Всякие correctansweris тоже скорее всего уже есть в файлах базовых вопросов. Нотации и движки уже есть в Preg, там я думаю вполне подходящие названия и т.д. Свои строки вводить тогда, когда это реально нужно (например тестовые строки имеют совсем другое сообщение в Preg чем у нас в подсказке ибо смысл их там другой; равно как там нет понятия "показать вашу/показать учителя" для подсказок).

  82. Камо Сперцян reporter

    К вопросу про 72d6e38 - результат от generateJson ничем не отличается от generateHtml, но есть один момент: для каждой подсказки код результата получается из разных полей json-объекта. Для описания это json["description"], для графа объяснения - json["graph"]["img"], для синтаксического дерева - json["tree"]["img"]. И если первый ключ я могу получить через json_key() инструмента, то как поступать со вторым я не знаю. Если мы можем изменять методы генерации json-а, то можно вообще убрать лишнее поле "img" и записывать результирующую строку во всех трех подсказках напрямую по ключу json_key(). В таком случае можно методы генерации подсказки вынести для всех трёх подсказок в базовый класс.

    И ещё одно но: для подсказки описания для обоих ответов (студента и преподавателя) я выводил результаты на той же строке, где пишется принадлежность подсказки (Описание Вашего ответа: ...). Для деревьев и графов вставляется разделитель после этой строки. Как мне быть в этом случае? Писать описание тоже с новой строки, чтобы не переопределять метод?

  83. Oleg Sychev repo owner

    Изменять generate_json нельзя (у него есть другие клиенты), можно менять только generate_html. И этого должно быть вполне достаточно для их унификации.

    Вообще нужный ключ можно возвращать из специальной функции, но можно для начала и оставить отдельно description а сэкономить в базовом классе только 2.

    По ответам не вижу проблем добавить перевод строки и перед описанием тоже, чтобы не сливалось с надписью чей это ответ. Может там даже абзац а не перевод строки нужен...

  84. Oleg Sychev repo owner

    Обратите внимание, граф и дерево через generate_json если unaccepting_regex скорее всего не в img возвращают, а в других индексах...

  85. Камо Сперцян reporter

    Олег Александрович, сейчас сижу и с языковыми константами разбираюсь. К слову, уже сейчас нашел много лишних строк. Мне интересно, могут ли какие-то константы writeregex быть использованы все самого плагина?

  86. Камо Сперцян reporter

    Обнаружил, что всё-таки некоторые константы, вроде наименования плагина, используются извне. Пока закомментировал все константы, которые собираюсь удалить, и проверяю, действительно ли они не используются.

    Возник такой вопрос: в форме редактирования вопроса при нажатии на help (вопросик) возле полей Penalty для хинтов выскакивают тултипы со следующими заголовками:

    • Syntax tree: prnalty

    • Explanation graph: fine

    • Explanation of the expression: fine

    • Test string: fine

    Помимо опечатки в слове prnalty для Syntax tree непонятна разница в слове после двоеточия. Почему у всех остальных написано fine? И как должен выглядеть правильный заголовок окна подсказки?

  87. Камо Сперцян reporter

    Доделал наконец эту не самую увлекательную работёнку. Разобрал языковые константы. Вроде все текущие замечания по PeerReview устранил. Все изменения вытолкнул.

  88. Oleg Sychev repo owner

    Там имеется очень немного констант извне - в основном для окна редактирования и названия плагина. Меньше десятка.

    По хелпу - это точно "заголовок окна подсказки" или это все-таки его содержимое? Если содержимое то надо внятным для пользователя образом пояснить смысл числа или что там вводится. Если это penalty то написать что это сколько вычитается за применение подсказки.

  89. Oleg Sychev repo owner

    Остальное посмотрю в ближайшие дни как будет время. Но врядли завтра.

    А работа она и есть работа. Наведение порядка что в доме перед праздниками, что в файлах со свалкой строк сильно приятным не бывает, зато потом работать удобнее. При прежней разработке за этим не очень следили, но там и задача другая была - все-таки с нуля немало кода сделано, представьте если бы вы разбирались и с нуля его писали, насколько это было бы дольше и тяжелее.

  90. Камо Сперцян reporter

    Это именно заголовок окна подсказки. Содержимое там другое. Для русского языка названия все одинаковые - Тип подсказки: штраф. А в английском то penalty, то вообще какой-то fine. Я полагаю, что надо везде сделать penalty.

  91. Камо Сперцян reporter

    Буду пока проверять код CodeChecker-ом и начну писать Testing Instructions.

  92. Oleg Sychev repo owner

    penalty. Fine это еще и штраф, но другой - ну типа например который ГАИ берет. Это товарисч посмотрел в словарь невнимательно...

  93. Камо Сперцян reporter

    Исправил. Проверил CodeChecker-ом и привёл в соответствие требованиям весь код. Всё вытолкнул.

  94. Камо Сперцян reporter

    Юнит-тесты сделал, кроме тестов к функциям qtype_writeregex_edit_form - там очень сложный конструктор, использующий много разной информации. Как посоветуете поступить? Описать тестирование вручную? В принципе, ручное тестирование не будет сильно сложным для этих функций.

  95. Oleg Sychev repo owner

    По тестированию формы посмотрите что-то типа preg/tests/questiontypetest.php и подключаемый из него preg/tests/helper.php - там делаются тесты с формой, только на ее сохранение в БД и потом считывание. Для валидации можно аналогично сделать...

  96. Oleg Sychev repo owner

    00cea55 - нарушение стандарта кодирования, функция getRegexOptions названа CamelCase - должна быть в нижнем регистре через подчеркивания. local_codechecker это не видит, он же слов не знает... Исправьте.

  97. Камо Сперцян reporter

    Local_codechecker мне ругался на это название, поэтому оно уже исправлено в коммите 9c19868.

  98. Oleg Sychev repo owner

    Для валидации написать юнит-тесты так и не получилось?

  99. Камо Сперцян reporter

    К сожалению, нет. Я попытался разобраться, но там как-то сложно всё.. Я описал, как проверять вручную, в принципе там ничего сложного.

  100. Oleg Sychev repo owner

    Там не столько сложно, сколько нудно каждый раз это вручную проделывать. Вы в описании кстати сильно сэкономили, не описывая конкретно что вводить и где ошибки ждать.

    Давайте я вам завтра на консультации покажу, там на самом деле все вовсе даже не сложно.

  101. Камо Сперцян reporter

    Хорошо. А бумажную версию пояснительной мне, получается, приносить пока не надо?

  102. Oleg Sychev repo owner

    Ну до пятницы доделаете если что. А что там менять то толком, разве что список коммитов? Так сделайте на отдельных листах с местом в конце, перепечатаете последний лист из него и все. В содержание коммитов у вас и без последних 10 страниц я думаю наберется...

    P.S. Я Тане Потаповой написал чтобы она тоже в 16 подошла потестить ее код, продублируйте на всяк. случай...

  103. Камо Сперцян reporter

    Вы имели ввиду до четверга, наверное? Мне на данный момент там надо что-то изменить? Или можно печатать, оставив место на возможный коммит?

    Тане передам.

  104. Oleg Sychev repo owner

    Я сейчас пытаюсь найти время код ваш потщательней посмотреть. Неужели надо ПЗ еще читать? Там сложно где-то сделать ошибки, все важные материалы мы заранее обсуждали а список коммитов и код в них - он сам по себе. В начале где описывается проблема и пути решения выпишите все устраненные проблемы - и каким образом они устранялись. Где у вас проблема в тестовых инструкциях я вам выше написал - очень обощенный характер в тех случаях, где надо вносить ошибки и т.д. По юнит-тестам завтра поговорим по всем, долго писать - пока скажу только обратить внимание на то, каким количеством тестов и на какие ситуации тестировалась get_best_fit_answer в preg - и сравните со своим планом тестирования.

    И еще я удивлен отсутствием юнит-тестов на compare_strings_analyzer - там есть чего тестировать и класс полностью автоматизирован, его тестировать очень легко.

  105. Камо Сперцян reporter

    Хорошо, я добавлю местами конкретики и посмотрю этот класс. Проблемы и пути решения я описал именно так, как Вы и сказали.

  106. Камо Сперцян reporter

    Вроде есть кое-какие успехи с тестами для формы - смог получить в каком-то виде форму, но такой вопрос наперед - какой путь мне надо указать до утилиты dot в тестах? Он же у каждого тестера будет отличаться. И как мне тестировать подсказки, использующие graphviz, под ArchLinux-ом?

  107. Oleg Sychev repo owner

    Ну оставьте одну конкретную ошибку об отсутсвии dot по указанному пути без тестирования, а пустой путь вы вполне можете протестить. Там других условий валидации для тестирования предостаточно...

  108. Камо Сперцян reporter

    Хорошо. С пустым и некорректным путями попробую сделать через этот global $CFG. Прочие валидации вроде не сулят проблем.

  109. Oleg Sychev repo owner

    Ну кстати некорректный путь проверить легко - задать заведомо некорректный.

    Сложнее если какую-то проверку нельзя сделать без корректно работающего dot.

  110. Камо Сперцян reporter

    Всё, сделал на все валидации тесты на все возможные случаи. Даже обнаружил и исправил одну серьёзную ошибку. Всё вытолкнул. Сейчас поправлю документ и продолжу-таки готовиться к зачёту.

  111. Камо Сперцян reporter

    Олег Александрович, у меня проблема с выводом расхождений пользователю. feedback формируется в функции рендерера

    public function feedback(question_attempt $qa, question_display_options $options)

    Но как отсюда получить расхождения? По логике, функция должна вызывать функцию feedback() анализаторов (для этого было бы неплохо все-таки выделить базовый класс анализатора). Здесь у меня 2 вопроса:

    • во-первых, где формировать ответ анализатора? В самом анализаторе или вызывать функцию рендерера?

    • во-вторых, откуда взять расхождения, если экземпляр анализатора создается в функции get_best_fit_answer и там же пропадает?

  112. Камо Сперцян reporter

    Вы просили написать в issue насчёт проблемы с установкой флага матчера для валидации переходов.

  113. Камо Сперцян reporter

    И ещё такой момент. Мы сегодня решили расхождения по тегам выдавать отдельно для открывающих и закрывающих. Но в случае, когда в двух автоматах есть переходы с такими тегами:

    o: 1, 3... c: 2, 4

    o: 1... c: 2, 4

    соответственно, выдача расхождений отдельно будет некорректна, т.к. закрывающие теги 2, 4 захватываются в обоих автоматах. Может, стоит на этапе проверки теги рассматривать отдельно - открывающие и закрывающие?

  114. Oleg Sychev repo owner

    Вы бы нумеровали свои вопросы когда выдаете несколько сразу, а то неясно - на какой отвечать.

    По поводу feedback - если брать опыт preg - рендерер вызывает best_fit_answer, которая возвращает кешированный результат. А там содержатся не только оценки, но и данные от анализаторов - мы вроде это добавляли уже? Или как минимум собирались добавить данные о расхождениях в ответ от best_fit answer. А на их основе уже выдаются сообщения. В preg эту роль играют matching_results

    По поводу сообщений - пример слишком простой, возможно. Разве не достаточно указать в сообщении только разницу в тегах? Сформулировать его так, чтобы подчеркнуть эту разницу, а совпадающие теги не упоминать?

  115. Камо Сперцян reporter

    Про feedback это вообще старый вопрос, мы с ним уже с ним разобрались.

    Нельзя хранить расходящиеся теги, т. к. при нескольких переходах у одного автомата неясно, с каким сравнивать. Например:

    o: 1, 3... c: 2, 4

    o: 1, 5... c: 2, 4

    У второго:

    o: 1, 3, 5... c: 2, 4

  116. Oleg Sychev repo owner

    Вытолкнул изменение в get_matcher, теперь есть отдельная get_matching_options - пользуйтесь. В коммите на примере файла из каталога authoring_tools видно, как ей пользоваться.

  117. Oleg Sychev repo owner

    По строкам надо нарисовать дерево сборки фразы. Подойдите в начале следующей недели с образцами самых сложных фраз - нарисуем...

  118. Камо Сперцян reporter

    Олег Александрович, нам тут в среду надо бы показать первую главу диплома. А что мне в неё писать?

  119. Oleg Sychev repo owner

    Вообще такое лучше письмом было писать, не в issue - или отдельное issue завести...

    И вообще - вы это в воскресенье что ли узнали? Ну и оперативность...

    Первая глава обычно самое замороченное, потому что требует отдельных работ - типа сравнения с аналогами. До среды это напряг...

    У вас как первая задача звучит? Вот примерно об этом должна быть первая глава - только с той разницей, что объяснить все надо с самого начала - зачем нужно обучать регулярным выражениям и кого, какие методы для этого есть и какие программные средства для автоматизации, сравнение их достоинств недостатков, постановка задачи на вашу разработку в конце.

    Пришлите перечень разделов с учетом формулировки первой задачи и перечисления выше - я откорректирую, если что не так.

    Тему я переслал для включения в приказ "Программный модуль тестового вопроса writeregex с ответом в виде регулярного выражения", но приказ пока не вышел так что точной гарантии что она изменится и именно так нет...

  120. Log in to comment