Improve documentation and error messages for better developer experience#1781
Draft
yarikoptic wants to merge 11 commits intomasterfrom
Draft
Improve documentation and error messages for better developer experience#1781yarikoptic wants to merge 11 commits intomasterfrom
yarikoptic wants to merge 11 commits intomasterfrom
Conversation
git-subtree-dir: .lad git-subtree-split: d765702eb44e88c33d93261ccb89139ca31cf5d1
- Add comprehensive testing guide to Sphinx docs (docs/source/development/testing.rst) - Add contributing guide to Sphinx docs (docs/source/development/contributing.rst) - Update CLAUDE.md with file placement guidelines to prevent root clutter - Update DEVELOPMENT.md to reference new Sphinx documentation - Add .lad/tmp/ to .gitignore for LAD session artifacts - Integrate development docs into main Sphinx documentation index This improves documentation organization and prevents temporary files from cluttering the project root. LAD-generated analysis files now go to .lad/tmp/, while permanent documentation goes to docs/source/. Testing documentation now properly integrated with Sphinx for better searchability and navigation.
- dandiapi.py: Document REST API client classes and main functionality - exceptions.py: Document custom exception hierarchy - consts.py: Document constants, metadata fields, and configuration
- download.py: Document download functionality and features - upload.py: Document upload workflow and validation - delete.py: Document deletion operations - move.py: Document move/rename functionality - organize.py: Document file organization and naming
- validate.py: Document validation functionality - pynwb_utils.py: Document NWB file handling utilities
Improve error messages in dandiset.py to provide actionable guidance: - Add hint about using 'dandi download' when dandiset.yaml is missing - Explain that metadata file may be empty or corrupted - Clarify required 'identifier' field in metadata structure These improvements help users quickly diagnose and fix common metadata issues. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Improve error messages in dandiapi.py to provide clearer guidance: - Explain mutually exclusive api_url/dandi_instance parameters - Add hint to verify Dandiset ID and check permissions - Reference 'dandi ls' command for listing assets - Include version context in "No asset at path" errors These improvements help users quickly troubleshoot API access issues. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Improve error messages in move.py to provide clearer guidance: - Explain why paths cannot navigate above Dandiset root - Add hints for file vs directory path requirements - Clarify "Cannot move current working directory" restriction - Reference 'dandi ls' command for checking remote paths - Explain dandiset.yaml requirement with actionable hints Update test assertions to match improved error messages. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Improve error messages in upload.py and download.py to provide clearer guidance: - Add file path context to "File not found" errors - Reference 'dandi validate' command for validation failures - Suggest checking file format and corruption for digest errors - Explain NWB specification requirements for metadata extraction - Provide example usage for download command when no URLs given These improvements help users quickly diagnose and fix upload/download issues. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace incorrect use of reduce() with direct os.path.commonprefix() call. The original code used reduce() incorrectly - os.path.commonprefix() already accepts a list of paths and returns the longest common prefix. This removes one type: ignore comment and improves code clarity. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1781 +/- ##
===========================================
- Coverage 75.11% 50.43% -24.68%
===========================================
Files 84 84
Lines 11921 11920 -1
===========================================
- Hits 8954 6012 -2942
- Misses 2967 5908 +2941
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
During one of the recent Zooms @chrisfoulon pointed to his https://github.com/chrisfoulon/LAD, which I decided to give a shot to. So far experience was quite fun! Unfortunately because of that this PR is a cocktail of things, such as adding LAD itself and then making it review codebase regarding testing and also propose and implement some minor enhancements. For better digestion I will
.lad/to make it easier to review etc,This PR enhances the developer experience through improved documentation and more actionable error messages. Changes were made systematically based on LAD (LLM-Assisted Development) test quality framework analysis, with each improvement committed separately and verified with the full test suite (548 tests, 100% pass rate).
Overview
Key Changes
1. Documentation Improvements (Commits: dedec0e, 5237428, 7494681, 15b1854)
Added comprehensive module docstrings to improve IDE support and code navigation:
dandiapi.py,exceptions.py,consts.py)download.py,upload.py,delete.py,move.py,organize.py)validate.py,pynwb_utils.py)Reorganized testing documentation:
docs/source/development/)CLAUDE.mdwith file placement guidelinesDEVELOPMENT.mdto reference new Sphinx docsFile organization:
.lad/tmp/for LAD session artifacts and temporary analysis files.lad/tmp/to.gitignoreSee commits:
2. Error Message Enhancements (Commits: 470467b, 3922285, 7c1788e, fb9ea70)
Made error messages more actionable with specific guidance on how to fix issues:
Dandiset metadata errors (
dandiset.py):dandi downloadAPI client errors (
dandiapi.py):dandi lscommand and includes version contextPath validation errors (
move.py):Upload/download errors (
upload.py,download.py):dandi validatecommand for detailsSee commits:
3. Code Quality Improvements (Commit: 928ad98)
Type annotation cleanup:
reduce()in upload sync path calculationos.path.commonprefix()call (which already accepts a list)type: ignorecommentSee commit:
928ad98c- Fix type annotation in upload sync path prefix calculationTesting
All changes verified with full test suite:
Each improvement category was tested independently before committing.
Additional Analysis Performed
As part of LAD framework analysis, the following were investigated but not changed:
Windows path handling (utils.py TODOs):
is_interactive()hasattr checks - current implementation works correctly on WindowsType annotations:
type: ignorecomments in codebaseLarge function refactoring:
organize.py:act()(248 lines) andfiles/bases.py:iter_upload()as candidatesCommit-by-Commit Details
For detailed information about each change, please review individual commits:
Each commit includes detailed descriptions and co-author attribution.
Files Changed
19 files changed: +692 insertions, -26 deletions
CLAUDE.md,DEVELOPMENT.md,.gitignore,docs/source/development/(new files)dandiapi.py,dandiset.py,exceptions.py,consts.pyupload.py,download.py,delete.py,move.py,organize.py,validate.pypynwb_utils.pytest_move.py(updated assertions to match improved error messages)Note: This PR also includes LAD (LLM-Assisted Development) framework files in
.lad/directory which add comprehensive test quality analysis workflows. These are development tools and do not affect runtime code.Related Issues
This work was motivated by achieving 100% meaningful test success and improving the developer experience. While no specific issues are directly closed, these improvements address common user friction points when errors occur.