Closed emartinm closed 3 years ago
ivarui01
commented
3 years ago ¿En que consiste exactamente esta tarea? es que creo que no termino de entenderla muy bien. A la hora de crear el problema, ¿habría que dar la opción de añadir diferentes bases de datos iniciales en las que la única diferencia es el valor de las filas de las tablas? y luego para el alumno, ¿se elegiría que base de datos inicial se usa a través de un botón o algo?
emartinm
commented
3 years ago Ahora mismo un problema SELECT consta de unas instrucciones DDL que crean tablas y otras instrucciones DML que insertan filas en esas tablas. Cuando un estudiante envía una solución, se ejecuta en ese estado de la BD y se comprueba que lo que se obtiene es lo mismo que con la solución oficial del profesor. Pero puede darse el caso de que una consulta SQL que está mal sí que funcione únicamente con ese estado de la BD que el estudiante conoce.
La idea de esta tarea es extender los problemas SQL para que puedan tener más de un conjunto de instrucciones DML, es decir, que existan varios estados de la BD sobre los que probar que la sentencia del estudiante genera los mismos resultados que la consulta oficial.
Para ello habrá que hacer algunas cosas:
ivarui01
commented
3 years ago Para el punto 3, ¿los alumnos pueden saber cuales son los estados iniciales de la BD o solo el oficial? Solo el oficial me imagino.
emartinm
commented
3 years ago En el enunciado únicamente verán el oficial porque si no ocuparía muchísimo, pero si su envío falla en el estado inicial número 6, la retroalimentación debería ser completa:
La idea es diferenciarse de DomJudge y que la retroalimentación les permita fácilmente detectar sus fallos.
jcorreas
commented
3 years ago Hola, estoy de acuerdo en la descripción detallada. Efectivamente el punto 3 es el más delicado para definir esta tarea. Estoy de acuerdo con Enrique en que el sistema debe dar ayudas al estudiante sobre las BD ocultas, y es algo que se puede hacer sin mucho esfuerzo.
En otra tarea (sobre el "modo examen") podemos definir más los casos en los que no se da ayuda al estudiante sobre los errores cometidos.
emartinm
commented
3 years ago Parece que no hay un campo concreto ListField pero creo que una lista de str se podría almacenar en un campo JSONField (https://docs.djangoproject.com/en/3.1/ref/models/fields/#jsonfield). Pero puede que sea algo exagerado usar un campo JSON solo para listas de str.
En el peor de los casos se podría dejar el modelo tal cual está pero considerar que el campo insert_sql puede contener varias bases de datos iniciales separadas por el separador (p.ej, una línea con -- @new data base@). Tiene la buena propiedad de que si un ejercicio tiene una única BD, su insert_sql no tendrá separador --> exactamente como hasta ahora. Y por uniformidad hacer una función/método split_initial_db(code) que recibe todo el texto de insert_sql y devuelve una lista de str con cada BD inicial separada.
@jcorreas , @ivarui01 , ¿qué os parece reutilizar el campo de texto entendiéndolo de esta otra manera? ¿Es muy cutre?
ivarui01
commented
3 years ago Yo creo que reutilizar el campo de texto esta bien, además puede que permita acceder de manera específica a cada una de las bases de datos iniciales mas fácilmente y con el JSONField tal vez se estaría complicando la estructura para algo que en principio es sencillo y no debería tener mucha complicación.
La función split_initial_db(code), ¿Sería una función fuera del modelo Problem?
jcorreas
commented
3 years ago Yo lo veo desde el punto de vista del profesor que posiblemente tiene en local los ficheros fuente de los ejercicios. Es muy cómodo tener un fichero con separaciones de ese tipo, y como dices no implica ningún cambio más allá de la operativa de cómo hacer las pruebas (que ya había que hacer en cualquier caso). Me parece muy bien que se haga así, con un solo fichero con anotaciones en comentarios.
Respecto a los alumnos, no recuerdo si al final iban a tener a su disposición el fichero con todas las bases de datos iniciales o solo la primera, la pública. En cualquier caso, creo que no hay mucho problema, excepto que hay que advertirles de que los conjuntos de insert son independientes.
emartinm
commented
3 years ago La función
split_initial_db(code), ¿Sería una función fuera del modelo Problem?
Pues ahora que lo dices, es aún más cómodo: tener un método insert_sql_list(self) en Problem que devuelva la lista de str con los inserts de cada BD inicial. No necesita parámetro porque lo obtiene del parámetro insert_sql que tienen todos los problemas. Si solo hay una BD inicial, lista unitaria. Y así lo único que hay que cambiar es donde se use directamente problem.insert_sql para cambiarlo por problem.insert_sql_list() y recorrerlo.
Respecto a los alumnos, no recuerdo si al final iban a tener a su disposición el fichero con todas las bases de datos iniciales o solo la primera, la pública. En cualquier caso, creo que no hay mucho problema, excepto que hay que advertirles de que los conjuntos de insert son independientes.
Si se deja el código como está ahora, se bajarían los create seguido de los insert y tendrían en un único fichero todas las BD separadas por el separador. Habría que avisarlo/documentarlo, pero sin tocar nada puede ser un funcionamiento bastante adecuado para los alumnos. Y si únicamente tienen una BD todo funciona exactamente igual que ahora.
Oye, cada vez me gusta más. Qué buena idea idea hemos tenido entre los 3 :-)
emartinm
commented
3 years ago He caído en una cosa de este enfoque que requiere de un tratamiento especial: los campos initial_db y expected_result de los problemas. El campo initial_db contiene un diccionario con la representación de la BD inicial para mostrarla en el enunciado sin tener que recalcular nada, mientras que expected_result contiene un diccionario con el resultado esperado de ese problema, que puede puede ser una tabla o varias tablas dependiendo del tipo de problema. En el momento que los problemas tienen una lista de BD iniciales que puede ser unitario, esos dos campos tienen que pasar a ser listas también en la BD y adaptar el código que las use para que sean listas.
Desde el punto de vista del modelo no hay problema porque ese campo es JSONField y puede almacenar listas de diccionarios, pero habrá que tenerlo en cuenta para actualizar los problemas que ya existen en la base de datos para que tu nuevo código funciones con los problemas. La actualización es sencilla (cambiar diccionarios por listas unitarias con ese diccionario) pero hay que hacerla con todos los problemas. Voy a implementar un método adapt_db_result_to_list() en el módulo shell.py para ejecutarlo en el servidor. @ivarui01 , tú puedes usarlo también si quieres, o también puedes borrar los problemas antiguos y volver a crearlos cuando hayas actualizado el clean de los problemas para crear initial_db y expected_result como listas.
emartinm
commented
3 years ago ivarui01
commented
3 years ago Yo también había estado pensando en lo de initial_db pero había estado intentando usarlo sin necesidad de guardarlo para no tener que cambiar lo que ya hay, porque realmente la initial_db de los inserts que no son el principal solo la necesito para mostrarla cuando el resultado coincida con la BD inicial principal pero no con alguna de las otras para que, como me dijisteis, el alumno sepa en que se ha equivocado.
En cuanto a expected_result, ¿no debería ser el mismo para todas las BD iniciales? ya que en principio el resultado tendría que ser el mismo para todas las BD iniciales porque entonces si no serian ejercicios diferentes, esta cuestión también me cuesta un poco entenderla porque no termino de ver en que situaciones se podrían poner varias BD iniciales, se me han ocurrido algunos casos fáciles poco útiles pero no se.
Y realmente la tarea la tendría ya hecha (sin contar los tests), lo único que me faltaría sería lo de mostrar la BD inicial que no es la principal en caso de que falle en una que no sea la principal, que creo que es lo más "difícil".
emartinm
commented
3 years ago Yo también había estado pensando en lo de
initial_dbpero había estado intentando usarlo sin necesidad de guardarlo para no tener que cambiar lo que ya hay, porque realmente lainitial_dbde los inserts que no son el principal solo la necesito para mostrarla cuando el resultado coincida con la BD inicial principal pero no con alguna de las otras para que, como me dijisteis, el alumno sepa en que se ha equivocado.
Sí, pero es conveniente guardarlo por eficiencia, ya que invocar a Oracle para calcularlo tarda en torno a 1 segundo. Es mucho mejor calcularlo una vez y almacenarlo para todas las siguientes. De hecho tanto initial_db como expected_results son innecesarios en el sentido de que siempre se podrían recalcular, pero es mejor dejar Oracle para corregir envíos y almacenar el resto para acelerar.
En cuanto a
expected_result, ¿no debería ser el mismo para todas las BD iniciales? ya que en principio el resultado tendría que ser el mismo para todas las BD iniciales porque entonces si no serian ejercicios diferentes, esta cuestión también me cuesta un poco entenderla porque no termino de ver en que situaciones se podrían poner varias BD iniciales, se me han ocurrido algunos casos fáciles poco útiles pero no se.
No, el resultado es lo que deberá producir el código del alumno para ser considerado correcto. Imagina el problema SELECT de encontrar el nombre de todos los clubes de Andalucía y tienes dos BD iniciales:
entonces expected_results será una lista de dos resultados diferentes, uno para para cada BD inicial:
Es decir, es un único ejercicio pero con cada BD inicial se espera un resultado diferente. Lo que sí que es única es la solución oficial que se almacena en el campo solution (quizá eso es lo que te ha liado).
Y realmente la tarea la tendría ya hecha (sin contar los tests), lo único que me faltaría sería lo de mostrar la BD inicial que no es la principal en caso de que falle en una que no sea la principal, que creo que es lo más "difícil".
Cuando se corrige un envío se devuelve un JSON con la información relevante: resultado, feedback... https://github.com/emartinm/lsql/blob/8d5bea30f5d367f91d6877227f4834f70e8b9bb8/lsql/judge/views.py#L335
El campo feedback contiene código HTML con la explicación de la retroalimentación, así que lo que tendrás que hacer es cambiar el método Problem.judge() para que devuelva toda la información en lugar únicamente una tabla con los resultados esperados. Básicamente, si se detecta un error en la primera BD se devuelve lo mismo que ahora, y si se detecta en alguna posterior hay que incluir la BD inicial que hay fallado junto con el resultado erróneo (básicamente es concatenar la BD inicial).
ivarui01
commented
3 years ago Ya he cambiado en todos los sitios donde se usa initial_dby expected_result por initial_db[0] y expected_result[0], tanto en model como en las vistas, etc (en los problemas de select van desde 0 hasta N). He actualizado los ejercicios dándole al botón SAVE para que se vuelva a ejecutar la función clean y los problemas funcionan bien (se muestran bien, las soluciones se suben bien), el problema es que ahora me falla cuando subo un ejercicio nuevo a traves del boton "+add" de Select problems y he estado mirando y no se muy bien por que es, me sale el siguiente erro tras añadir el zip y seleccionar la colección y el usuario:
En la consola no sale nada sospechoso
Y si lo intento cargar desde el "+add" de problem me sale esto:
emartinm
commented
3 years ago Los problemas se deben añadir en la clase específica, en este caso SelectProblem, así que te puedes olvidar del segundo error. Con respecto al primero, ¿si en lugar de un ZIP pones los valores a mano sí que se añade correctamente? Como la excepción es capturada por la interfaz admin al final la única poca información que hay es esa, pero es posible que sea un problema con el fichero ZIP. ¿Puedes compartirlo por aquí para que podamos verlo?
ivarui01
commented
3 years ago Si pongo los valores a mano ocurre lo mismo. El zip es el mismo de siempre. select_ok.zip
ivarui01
commented
3 years ago Vale, haciendo otra parte de la tarea he llegado a la solución.
El problema es que a la hora de hacer el clean lo estaba haciendo asi: self.initial_db[0] = res['db'], y me he dado cuenta que cada vez que actualizaba el ejercicio, el contenido de initial_db y expected_result se estaba duplicando porque para ir añadiendo el resto de BD iniciales lo hago con append() pero self.initial_db[0] = res['db'] no borraba lo anterior, simplemente cambiaba lo de la posición "0", por lo que se iban añadiendo a continuación de las que ya había. Entonces al añadir un ejercicio nuevo, self.initial_db[0] no existe y por eso daba error, porque no había podido acceder a la posición "0".
Ahora el problema es que no puedo editar el problema porque cuando le doy a "SAVE" no se guarda nada, a ver si lo soluciono.
emartinm
commented
3 years ago Esto puede ser útil también para @Tamiflu9 y @Dashito14 .
A veces al usar el interfaz admin o las propias páginas del juez ocurren fallos pero no obtenemos demasiada información o nos gustaría hacer pruebas cambiando algunos valores a mano y no podemos. Para estas circunstancias os recomiendo que os conectéis al terminal de Python, importéis los módulos que necesitáis y creéis objetos, realicéis consultas de prueba... lo que necesitéis.
Para acceder al terminal de Python completamente configurado tenéis que ejecutar:
$ python manage.py shellSi se lanza alguna excepción obtendréis la traza completa, y podréis hacer todas las pruebas que necesitéis de manera sencilla.
ivarui01
commented
3 years ago En principio ya funciona todo, lo único que tengo la duda de dónde concatenar las tablas de las BD iniciales secundarias.
De primeras lo he hecho en el método Problem.judge() y quedaría así(los comentarios los quitare, solo los puse para enterarme por si me olvidaba):
¿Sería mejor añadir un nuevo parámetro a compare_select_results() y a su vez a feedback_rows() con la BD inicial secundaria en caso de que el error se encuentre en una secundaria y ahí concatenar las tablas no?
Ya que es aquí donde se rellena feedback:
El resultado es algo así:
¿Debería crear un html nuevo para añadir el párrafo <p><strong>Siendo la base de datos inicial la siguiente</strong></p> de igual manera que los que ya existen? Como por ejemplo:
emartinm
commented
3 years ago Los comentarios son bastante informativos, creo que están bien. Tradúcelos a inglés y déjalos, porque creo que son muy positivos.
El primer código que pegas está en SelectProblem.judge y no en Problem.judge, ¿verdad? Lo digo porque si usas execute_select_test no podrías estar en otro tipo de problema.
Me parece bien tu idea de añadir un nuevo parámetro a las funciones compare_select_results() y feedback_rows() y que sea initial_db=None si no hay que añadir la base de datos al feedback producido (es decir, es la base de datos que aparece en el enunciado). En otro caso, tendrá la información de la BD inicial que hay que añadir al feedback.
También creo que sería interesante mover algo de trabajo a la plantilla HTML, aunque quizá no haya que crear una nueva. ¿Se puede añadir únicamente un nuevo parámetro a la plantilla y que si es None entonces no se añada la base de datos al mensaje HTML, y si no ese parámetro contendría la información de la BD que ha fallado?
A lo mejor cambiaría el mensaje "Siendo la base de datos inicial la siguiente" por "Base de datos sobre la que se ejecutó tu código SQL" o algo así. No encuentro ninguna frase que me encante.
ivarui01
commented
3 years ago Si, el primer código es de SelectProblem.judge, me he equivocado.
Si podría añadir un nuevo parámetro a la platilla para imprimir las tablas, pero tendría que repetir ese mismo código en cada una de las plantillas de los diferentes tipos de errores, para cuando fallan las cabeceras de las tablas, faltan tablas, falla el orden y faltan filas, ¿O doy por hecho que en algunas de ellas siempre que falle en una BD inicial secundaria ha tenido que fallar si o si en la BD principal y por tanto ese error siempre va a saltar con la BD principal y no con una secundaria? Como por ejemplo en el caso de que falten columnas en el resultado, ya que siempre que falten en una BD secundaria también van a faltar en la BD principal ya que tienen que ser las mismas. No se del todo si esta afirmación que acabo de hacer es del todo correcta porque todavía tengo dudas de todos los posibles casos de ejercicios con varias BD iniciales, aunque, ¿Podría haber BD iniciales con tablas que tengan diferentes columnas que no se utilicen a la hora de obtener el resultado para así probar casos que ahora mismo no se me ocurren?
emartinm
commented
3 years ago Sí, habría que modificar todas esas plantillas, pero creo que ha llegado el momento de usar herencia de plantillas. Crea una feedback_wa.html que tiene dos bloques, uno primero feedback sin definir y otro segundo bd que mira un parámetro initial_db y si es diferente de None muestra un mensaje seguido de la BD que ha fallado.
Una vez tengas esta, el resto de plantillas feedback_wa_*.html heredarán y únicamente redefinirán el primer bloque para mostrar el contenido que actualmente están mostrando.
emartinm
commented
3 years ago No olvides subir tus cambios a tu repositorio remoto para que pueda mirar el error que has comentado. Y si me puedes pasar toda la salida de tus tests hasta llegar al error y adjuntarla como fichero de texto también te lo agradeceré.
emartinm
commented
3 years ago Mientras lo reviso y te digo, creo que podrás evitar ese error y seguir con los tests si comentas la línea https://github.com/emartinm/lsql/blob/ab3edce215b768c1fffbe451a5317a9247e8a6ba/lsql/judge/oracle_driver.py#L497 y la cambias por
pos = 0No hace falta que lo mires, acabo de ver lo que ocurría, es un error tonto al crear la BD en el test.
ivarui01
commented
3 years ago Y para hacer la comprobación de que feedback contiene <p><strong>Base de datos utilizada para la ejecución de tu código SQL:</strong></p> y las cabeceras de las BD iniciales, ¿no podría hacer esto, ya que feedback es un string?
emartinm
commented
3 years ago No hace falta que lo mires, acabo de ver lo que ocurría, es un error tonto al crear la BD en el test.
Perfecto!
Y para hacer la comprobación de que feedback contiene
<p><strong>Base de datos utilizada para la ejecución de tu código SQL:</strong></p>y las cabeceras de las BD iniciales, ¿no podría hacer esto, ya que feedback es un string?
Podrías, pero te iba a pedir que estructurases un poco más el HTML de feedback con dos partes: un div result con el restultado erróneo y otro div bd con la BD utilizada (este div puede estar vacío). Así, el test debería comprobar que el texto del div bd está vacío en un caso y contiene los nombres de las tablas en el otro. Aparte de para separar ambos casos, la división en dos bloques div nos servirá para aplicar estilos diferenciados si alguna vez lo necesitamos.
Otro test que querría que metieras, si no lo has hecho ya, es cargar un problema SELECT desde un fichero ZIP que tiene 2 o 3 bases de datos iniciales, y compruebes que tras cargalo el método que devuelve la lista de INSERT tiene el número de elementos esperado.
Tarea para @ivarui01
Permitir varias BD iniciales como casos de prueba en los ejercicios (actualmente solo se considera una BD inicial). Principalmente para problemas de tipo SELECT.