Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces three "hotfixes" to address specific issues in the grading system. The changes aim to resolve problems with package installation, improve coverage reporting output, and support pygame-based projects in pylint checks.
Changes:
- Modified package installation to use editable mode to fix directory structure renaming issues in pyproject.toml
- Added --no-skip-covered flag to coverage report commands to show all files including those with 100% coverage
- Added pygame to pylint's extension package whitelist to prevent false positives when grading pygame projects
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| grader/utils/virtual_environment.py | Changes pip install to use editable mode (-e flag) with explanatory comment |
| grader/utils/constants.py | Adds --no-skip-covered flag to both coverage report argument constants |
| grader/checks/pylint_check.py | Hardcodes pygame in pylint's extension-pkg-whitelist to avoid C extension warnings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Editable install fixes issues with directory structure renaming in pyproject.toml | ||
| output = run([pip_path, "install", "-e", project_path]) |
There was a problem hiding this comment.
The editable install mode (-e flag) creates a link to the project directory instead of copying files. This changes the behavior of package installation and could have implications for the grading process. While editable installs solve the issue mentioned in the comment about directory structure renaming, they may cause issues if the virtual environment outlives the project directory or if the project modifies itself during grading. Additionally, editable installs typically require a setup.py or pyproject.toml with setuptools configuration, which may not be present in all student projects. Consider documenting this behavioral change or providing a flag to make it optional.
|
|
||
| logger.debug("Running pylint check on files: %s", pylint_args) | ||
| pylint_args.append("--fail-under=0") | ||
| pylint_args.append("--extension-pkg-whitelist=pygame") |
There was a problem hiding this comment.
Hardcoding the pygame extension package whitelist applies this setting to all projects, even those that don't use pygame. This could mask legitimate pylint warnings for projects using other C extension packages. Consider making this configurable through the pylintrc file or as a constructor parameter to allow per-project customization.
No description provided.