-
Notifications
You must be signed in to change notification settings - Fork 77
Output visualizer #296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Output visualizer #296
Conversation
This is not ready to be merged. Known issues:
Some added thoughts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a quick look before I noticed there was already feedback from @Matistjati that needs to be addressed, so I'm holding off looking in more detail. Luckily, I think the comments I added do not overlap with his.
|
||
def main(): | ||
if not len(sys.argv) == 3: | ||
print("Usage: output_visualizer.py <submission_output> <feedback_dir>") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem to match the standard? https://www.kattis.com/problem-package-format/spec/2023-07-draft.html#invocation-2
|
||
Example of created file structure when run on different: | ||
different_images | ||
├── different.c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if I have different.c both as an accepted and as a wrong_answer submission?
self.warning(f'Wrong amount of visualizers. \nExcpected: 1\nActual: {len(self._visualizer)}') | ||
|
||
# Checks if a file's extension is allowed, and if so validates its header | ||
def check_is_valid_image(self, file) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are libraries and standard tools to do this type of file type detection. I'd prefer we use them, if possible, instead of re-implementing that logic ourselves. There's a python package called python-magic
. (An uglier option would be to just use file --mime-type
from the shell, but that feels hacky).
This feels like a problem in the standard to me. How is one even intended to sanely write an output visualizer? The only option I can see is to dump a huge dependency in the validator directory. And, as you point out, we definitely do not want to do that in our examples folder.
To do this safely, you'd need to include the imagemagick source (i.e., same issue as above with using PIL). Sure, problemtools may ensure it's there as a dependency, but there's no such guarantee for sandboxes on judge systems (and at some point, problemtools should also sandbox its runs). Examples included in problemtools should be good examples for users to base stuff off of, so it's not a good sign for the standard if we can't come up with a good example for an output visualizer. |
Good point, the examples should actually work outside of problemtools. @elzorroartico I've thought more about it, and I think we do want to run the output visualizers on the secret data. Currently, the structure is basically I propose changes in the In general, I also think we need to be more careful with filenames. Some annoying examples:
Perhaps the solution is to prepend a unique counter to all filenames? Although at that point, the order is going to be nondeterministic due to multithreading. I can't think of a better solution than to keep |
No description provided.