Rename drawing areas to cells #47
Conversation
- core: rename `fill_area` keywords to `fill_cell` - core: rename `_Area` to `_Box` - CLI: rename the `-a`/`--fill-area` option to `-c`/`--fill-cell`
Reviewer's Guide by SourceryThis pull request renames "drawing areas" to "cells" throughout the codebase, affecting both the internal implementation and the public API. The changes are primarily focused on variable and function naming, with some adjustments to documentation and comments to reflect the new terminology. Updated class diagram for PictureShowclassDiagram
class PictureShow {
+_position_and_size(pic_size, cell_size, stretch_small, fill_cell)
+_cells(layout, page_size, margin)
}
class _Box {
+x
+y
+width
+height
}
PictureShow --> _Box
note for PictureShow "Methods renamed to use 'cell' instead of 'area'"
note for _Box "Renamed from _Area to _Box"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @mportesdev - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 2 issues found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| cell_width = (page_width - (num_columns + 1) * margin) / num_columns | ||
| cell_height = (page_height - (num_rows + 1) * margin) / num_rows |
There was a problem hiding this comment.
suggestion (bug_risk): Use math.floor() to ensure integer values for cell dimensions
The calculation of cell_width and cell_height could result in floating-point values. Consider using math.floor() to ensure integer values, which could prevent potential issues with pixel-perfect layouts.
import math
cell_width = math.floor((page_width - (num_columns + 1) * margin) / num_columns)
cell_height = math.floor((page_height - (num_rows + 1) * margin) / num_rows)
| -m, --margin MARGIN set width of empty space around the cells containing | ||
| pictures; default is 72 (72 points = 1 inch) | ||
| -s, --stretch-small scale small pictures up to fit cells | ||
| -c, --fill-cell fill cells with pictures, ignoring the pictures' |
There was a problem hiding this comment.
suggestion (documentation): Consider highlighting the change from -a to -c option
This is a breaking change that might affect existing scripts. It might be helpful to add a note about this change in a prominent place in the README, in addition to the changelog entry.
| -c, --fill-cell fill cells with pictures, ignoring the pictures' | |
| -c, --fill-cell fill cells with pictures, ignoring the pictures' | |
| aspect ratio (formerly -a option) |
| --> | ||
| ### Breaking changes | ||
|
|
||
| - Library API: `fill_area` keyword parameters renamed to `fill_cell` |
There was a problem hiding this comment.
nitpick (documentation): Typo: 'parameters' should be 'parameter'
| for cell in cells: | ||
| assert cell.x == margin | ||
| assert cell.width == expected_width |
There was a problem hiding this comment.
issue (code-quality): Avoid loops in tests. (no-loop-in-tests)
Explanation
Avoid complex code, like loops, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| for cell in cells: | ||
| assert cell.y == margin | ||
| assert cell.height == expected_height |
There was a problem hiding this comment.
issue (code-quality): Avoid loops in tests. (no-loop-in-tests)
Explanation
Avoid complex code, like loops, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| for cell in cells[::3]: | ||
| assert cell.x == margin |
There was a problem hiding this comment.
issue (code-quality): Avoid loops in tests. (no-loop-in-tests)
Explanation
Avoid complex code, like loops, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| for cell in cells[6:]: | ||
| assert cell.y == margin |
There was a problem hiding this comment.
issue (code-quality): Avoid loops in tests. (no-loop-in-tests)
Explanation
Avoid complex code, like loops, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
fill_areakeywords tofill_cell_Areato_Box-a/--fill-areaoption to-c/--fill-cell