Skip to content

Commit 16d5060

Browse files
mvanwykclaude
andcommitted
docs: address code review suggestions from Claude
Implements 5 improvements based on PR review feedback: 1. Clarified test class organization wording (line 59) - Changed "for modules or classes" to "group tests by module or class being tested" - Removes ambiguity about organizational intent 2. Enhanced vectorization guidance with anti-patterns (lines 33-35) - Added warning against converting Series to lists - Added warning against using .iterrows() - Emphasizes using vectorized operations directly 3. Clarified "public APIs" in input validation (lines 38-39) - Changed "public APIs" to "public functions and class methods" - More specific and actionable 4. Added pytest fixtures guidance (line 70) - Encourages using fixtures for shared test data - Reduces duplication and improves readability 5. Added positive example for vague comparison assertions (lines 84-86) - Shows bad: checking if shapes differ - Shows good: verifying actual filtering behavior with specific assertions - Uses proper vectorized pandas operations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 95bea71 commit 16d5060

1 file changed

Lines changed: 9 additions & 5 deletions

File tree

CLAUDE.md

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,12 @@
3131
- Type annotations should use Python 3.11 formats
3232
- Remove unnecessary trailing whitespace
3333
- Use vectorized operations for numpy arrays and pandas DataFrames/Series wherever possible instead of loops or
34-
.apply() to keep code clean and performant
34+
.apply() to keep code clean and performant. Avoid converting Series to lists for iteration or using .iterrows() -
35+
use vectorized operations directly on the DataFrame/Series
3536
- Extract magic numbers as named constants with descriptive names (e.g., `GREEN_THRESHOLD = 1.0` instead of hardcoded
3637
`1.0` in conditionals)
37-
- Add input validation for public APIs: validate types, ranges, and constraints early with clear error messages
38+
- Add input validation for public functions and class methods: validate types, ranges, and constraints early with clear
39+
error messages
3840
- Use ternary operators for simple conditional assignments (e.g., `status = "active" if count > 0 else "inactive"`
3941
instead of multi-line if/else blocks)
4042

@@ -54,7 +56,7 @@
5456

5557
### Required Practices
5658

57-
- Prefer test classes for organizing related tests for modules or classes
59+
- Prefer test classes for organizing related tests (group tests by module or class being tested)
5860
- Always put imports at the top of the module (never import inside test methods)
5961
- Where possible prefer pandas `assert_frame_equal` to asserting individual values of the dataframe
6062
- This may require creating an expected dataframe to compare against
@@ -65,6 +67,7 @@
6567
- Include boundary/edge case tests for threshold values, limits, and special cases
6668
- When testing against expected values (colors, formats, etc.), reference package constants rather than hardcoding
6769
values in tests
70+
- Use pytest fixtures for shared test data setup to improve readability and reduce duplication
6871

6972
### Anti-Patterns to Avoid
7073

@@ -78,8 +81,9 @@
7881
- Only `assert hasattr()` or `callable()` checks without behavior verification
7982
- Only `assert isinstance()` type checks without validating actual behavior
8083
- Assertions that can never fail (e.g., `assert x >= 0 or x < 0`)
81-
- Vague comparison assertions (e.g., `assert result_with_filter != result_without_filter` instead of asserting what
82-
the filter actually does to the data)
84+
- Vague comparison assertions that only check "something changed" (e.g., `assert filtered_df.shape !=
85+
unfiltered_df.shape`). Better: verify the actual filtering behavior (e.g., `assert len(filtered_df) == 5` and
86+
`assert (filtered_df['price'] > 100).all()`)
8387
- **Don't duplicate tests**: Multiple tests with identical structure should use parametrize instead
8488
- **Don't over-mock**: Mock external dependencies only (databases, APIs, file I/O, network calls). Never mock internal
8589
PyRetailScience logic or calculations. Test real package behavior.

0 commit comments

Comments
 (0)