Skip to content

Migrate from nbdev to MkDocs, remove GroupEng dependency#18

Merged
scott-yj-yang merged 24 commits intomainfrom
scott/deprecate_nbdev
Feb 28, 2026
Merged

Migrate from nbdev to MkDocs, remove GroupEng dependency#18
scott-yj-yang merged 24 commits intomainfrom
scott/deprecate_nbdev

Conversation

@scott-yj-yang
Copy link
Collaborator

@scott-yj-yang scott-yj-yang commented Feb 28, 2026

Summary

This PR modernizes CanvasGroupy from an nbdev notebook-first project into a standard Python package with proper documentation and comprehensive test coverage.

Breaking changes

  • Removed GroupEng dependency -- AssignGroup now accepts CSV files or DataFrames via load_groups() instead of relying on GroupEng
  • Python >= 3.12 required (dropped 3.9-3.11)
  • Version bumped to 0.1.0 (previous main tagged as v0.0.1)

Package & build

  • Migrated to pyproject.toml with hatchling build backend
  • Updated LICENSE from MIT to Apache-2.0
  • Rewrote README with usage examples for all 4 classes
  • Added **repo_kwargs pass-through to AssignGroup.create_github_group()

Documentation (MkDocs Material)

  • Replaced Quarto/nbdev with MkDocs Material + mkdocstrings + mkdocs-jupyter
  • Added Google-style docstrings to all public classes and methods
  • Auto-generated API reference pages for AssignGroup, CanvasGroup, GitHubGroup, Grading
  • Getting-started guides (installation, authentication, quickstart)
  • Tutorial pages for Canvas groups, GitHub repos, grading, and feedback
  • Jupyter notebook tutorial with pre-computed outputs (assign-groups-demo)
  • CI workflow updated to deploy MkDocs instead of Quarto

Test suite (76 tests, all mock-based)

  • tests/conftest.py -- shared fixtures for Canvas, GitHub, and credentials
  • tests/test_assign.py -- 14 tests: load groups, create Canvas/GitHub groups
  • tests/test_canvas.py -- 25 tests: auth, course setup, email lookup, group operations, grade posting, conversations
  • tests/test_github.py -- 22 tests: auth, repo operations, collaborators, teams, file ops, issues, invitations
  • tests/test_grading.py -- 15 tests: issue fetching, score parsing, graded checks, grade posting, end-to-end workflow

All tests use unittest.mock -- zero real API calls, zero credentials needed on CI.

Cleanup

  • Deleted nbs/, dev_nbs/, test_nbs/ directories
  • Removed nbdev artifacts (_modidx.py, _proc/, settings.ini, setup.py, etc.)
  • Removed old tests/canvas_test.py placeholder

Test plan

  • uv run pytest tests/ -v passes (76 tests in 0.25s)
  • uv run mkdocs build --strict succeeds
  • uv run mkdocs serve renders correctly locally
  • GitHub Pages deploys after merge
  • Package installs without GroupEng: pip install -e .

🤖 Generated with Claude Code

scott-yj-yang and others added 21 commits February 27, 2026 11:44
Replace nbdev notebook-first workflow with a standard Python package structure
to enable agentic coding workflows. Source .py files are now the canonical code,
Quarto docs are kept as-is for the documentation site.

- Add pyproject.toml with hatchling build backend (replaces setup.py + settings.ini)
- Clean nbdev markers from source .py files and add missing import os in assign.py
- Remove nbdev artifacts: _modidx.py, _proc/, .gitattributes, .gitconfig
- Remove nbdev config: settings.ini, setup.py, MANIFEST.in, environment.yml
- Clean nbdev directives from API and tutorial notebooks
- Inline nbs/nbdev.yml metadata into nbs/_quarto.yml
- Replace CI workflows: nbdev-ci -> uv+pytest, quarto-ghp -> direct Quarto render
- Update .gitignore and README.md

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pandas 3.0 and NumPy 2.4 both require Python 3.11+. Bumping to 3.12
aligns with current ecosystem and keeps the project on supported versions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…reate_github_group

- Remove unused `import os` from assign.py
- Add TestCreateCanvasGroup: verifies assign_canvas_group calls, suffix
  handling, explicit group category, and empty-groups ValueError guard
- Add TestCreateGitHubGroup: verifies create_group_repo calls, missing
  username handling, and empty-groups ValueError guard
- Rename TestAssignGroupFromDataFrame to TestLoadGroups for clarity

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add **repo_kwargs to create_github_group() so callers can forward
  repo_template, description, team_slug, team_permission, etc.
- Remove unused `call` import from test_assign.py
- Add test for constructor `groups` parameter auto-loading
- Add test for create_canvas_group with group_category=None but
  explicit in_group_category provided
- Add test verifying **repo_kwargs are forwarded to create_group_repo

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Notebooks with pre-computed outputs can now be used as documentation
pages alongside markdown, giving the same output-style rendering
that nbdev/Quarto provided.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nd credentials

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…conversations

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…flow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@scott-yj-yang
Copy link
Collaborator Author

Code review

Found 3 issues:

  1. assign.py: create_github_group iterates self.groups (loaded from CSV via load_groups()), where the values are raw student_id entries from the CSV. These are used as keys into github_usernames (returned by fetch_username_from_quiz), which is keyed by email prefixes (SIS Login IDs). If the CSV student_id column contains Canvas numeric IDs rather than email prefixes, every lookup at line 126 will silently fail via the KeyError catch, and all repos will be created with zero collaborators. The contract between load_groups() and create_github_group() needs clarification -- either document that student_id must be an email prefix, or add a mapping step. (bug)

repos = []
for group_name, members in self.groups.items():
group_git_usernames = []
for email in members:
try:
group_git_usernames.append(github_usernames[email])
except KeyError:
print(f"{email}'s GitHub Username not found")

  1. github.py: In release_feedback, the except Exception block at line 443 catches a failed get_repo call and prints a warning, but execution falls through to self.create_issue_from_md(repo, ...) on line 445 which is outside the try/except. If the repo lookup fails on the first iteration, repo is undefined (NameError). On subsequent iterations, repo retains the previous iteration's value, silently posting feedback to the wrong repository. The except block needs a continue statement. (bug)

continue
try:
repo = self.org.get_repo(repo_name)
except Exception:
print(f"Repo: {bcolors.WARNING}{repo_name} NOT FOUND!{bcolors.ENDC}")
self.create_issue_from_md(repo, os.path.join(feedback_dir, repo_name, md_filename))

  1. pyproject.toml: requests is imported and used in canvas.py (fetch_username_from_quiz makes requests.get() calls at lines 427 and 437), but requests is missing from the [project] dependencies list. Since this PR introduces pyproject.toml as the canonical install manifest, any user who installs the package and lacks requests in their environment will hit an ImportError at runtime. (bug)

dependencies = [
"pandas",
"numpy",
"PyGithub",
"canvasapi",
]

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

scott-yj-yang and others added 3 commits February 27, 2026 16:48
1. assign.py: add UserWarning when load_groups() detects numeric
   student_id values (likely Canvas IDs instead of email prefixes),
   and clarify docstring that student_id must be the SIS Login ID.

2. github.py: add missing `continue` in release_feedback() exception
   handler so a failed repo lookup skips to the next repo instead of
   using an undefined/stale variable.

3. pyproject.toml: add missing `requests` to dependencies (used by
   canvas.py fetch_username_from_quiz).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@scott-yj-yang scott-yj-yang merged commit e4c79c3 into main Feb 28, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant