PythonTA Demo Website#1357
Conversation
Coverage Report for CI Build 27661606537Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage remained the same at 90.929%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
david-yz-liu
left a comment
david-yz-liu
left a comment
There was a problem hiding this comment.
@a1-su nice work! I left a few inline comments, and in response to your two questions:
- If you'd like to add to this to spawn a webworker for the
checkAllcall that's good with me - I noticed this issue a while ago with watchdog, so I've actually pulled it out, but this change hasn't been released yet. For now you can leave a comment saying that the watchdog mocking is necessary until python-ta 2.13 is released.
| <html> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <style> |
There was a problem hiding this comment.
please create a separate css file for this
| </div> | ||
|
|
||
| <div id="output-container"> | ||
| <div style="color: #666">Report will appear here.</div> |
There was a problem hiding this comment.
Please move this style into the CSS as well
| `) | ||
|
|
||
| const generateReport = pyodide.runPython(` | ||
| import python_ta |
There was a problem hiding this comment.
I suppose there's some tradeoff here with loading time. I believe if you move the import statement to inside generate_report the initial loading of the page will be a bit faster, but the generation of the report will be a bit slower.
Can you experiment with some profiling to see if this is the case? My intuition is we should move the import to generate_report (or, perhaps get rid of this function and just call pyodide.runPython directly in checkAll)
| checkAll = (codeInput) => { | ||
| pyodide.FS.writeFile("code_input.py", codeInput) | ||
| generateReport() | ||
| const htmlOutput = pyodide.FS.readFile("report.html", { encoding: "utf8" }) |
There was a problem hiding this comment.
Rather than create a separate file, can we instead redirect stdout?
| pyodide.FS.writeFile("code_input.py", codeInput) | ||
| generateReport() | ||
| const htmlOutput = pyodide.FS.readFile("report.html", { encoding: "utf8" }) | ||
| const safeHtmlOutput = htmlOutput.replace(/"/g, """) |
There was a problem hiding this comment.
I'm not exactly sure what the purpose of this line is. We do sanitization at the user-input level in the HTMLReporter already; but also if this is just about escaping HTML characters there are more that we would want to escape than just ".
david-yz-liu
left a comment
david-yz-liu
left a comment
There was a problem hiding this comment.
Hi @a1-su, thanks for making these changes! Can you please close this PR and instead make a new PR to add the files to the top-level of this new repo: https://github.com/pyta-uoft/pyta-sandbox.
We'll work on iterating on this new repo in the future!
3 participants
Proposed Changes
This PR introduces a demo website involving a single web page where the user can enter code and have it checked by PythonTA. The output then gets displayed on the web page after PythonTA is run with Pyiodide.
Screenshots of your changes (if applicable)
Type of Change
(Write an
Xor a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]into a[x]in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request:
Questions and Comments
(Include any questions or comments you have regarding your changes.)
watchdogwas the only one which was both not entirely in Python and also does not have a correspondingwhlversion available to import it with, so it had to be mocked.