This repository was archived by the owner on Apr 11, 2025. It is now read-only.
Simplify and fix Table.set_span #165
Closed
Conversation
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
bosd
added
refactoring
In `Table.set_span` there are a bunch of redundant checks for borders and number of bordered edges that this commit simplifies. Additionally the vertical and horizontal spans are independent of each other, therefore these checks can be untangled. I think that this actually uncovered a bug in the old implementation. For the cases where both horizontally and vertically only one of both edges are bordered there are differences with this commit. | left | right | top | bottom | vspan/hspan before | vspan/hspan after | | ----- | ----- | --- | ------ | ------------------ | ----------------- | | true | false | true | false | false | true | | true | false | false | true | false | true | | false | true | true | false | false | true | | false | true | false | true | false | true | This change in behaviour of the `Table.set_span` method is a bug fix. I guess in the original code it was missed that a cell could both horizontally and vertically span to other cells if the two bordered edges are not on the same direction.
The attributes `vspan` and `hspan` depend only on other attributes inside `Cell`, therefore there is no need to use a mutating method `set_span` to recover the invariants of `Cell`.
The condition `hspan and not left` expands to `(not left or not right) and not left` which simplifies to `not left`. After this change `Cell.hspan` and `Cell.vspan` aren't used anymore in this codebase. They stay because they are part of the API.
bosd
force-pushed
the
refactor-table-set-span
branch
from
0549e60 to
6118c3c
Compare
Bumps [cryptography](https://github.com/pyca/cryptography) from 43.0.1 to 43.0.3. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](pyca/cryptography@43.0.1...43.0.3) --- updated-dependencies: - dependency-name: cryptography dependency-type: indirect update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [virtualenv](https://github.com/pypa/virtualenv) from 20.26.6 to 20.27.0. - [Release notes](https://github.com/pypa/virtualenv/releases) - [Changelog](https://github.com/pypa/virtualenv/blob/main/docs/changelog.rst) - [Commits](pypa/virtualenv@20.26.6...20.27.0) --- updated-dependencies: - dependency-name: virtualenv dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [mypy](https://github.com/python/mypy) from 1.12.0 to 1.12.1. - [Changelog](https://github.com/python/mypy/blob/master/CHANGELOG.md) - [Commits](python/mypy@v1.12.0...v1.12.1) --- updated-dependencies: - dependency-name: mypy dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [virtualenv](https://github.com/pypa/virtualenv) from 20.26.6 to 20.27.0. - [Release notes](https://github.com/pypa/virtualenv/releases) - [Changelog](https://github.com/pypa/virtualenv/blob/main/docs/changelog.rst) - [Commits](pypa/virtualenv@20.26.6...20.27.0) --- updated-dependencies: - dependency-name: virtualenv dependency-type: indirect update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [mypy](https://github.com/python/mypy) from 1.12.1 to 1.13.0. - [Changelog](https://github.com/python/mypy/blob/master/CHANGELOG.md) - [Commits](python/mypy@v1.12.1...v1.13.0) --- updated-dependencies: - dependency-name: mypy dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [pydata-sphinx-theme](https://github.com/pydata/pydata-sphinx-theme) from 0.15.4 to 0.16.0. - [Release notes](https://github.com/pydata/pydata-sphinx-theme/releases) - [Changelog](https://github.com/pydata/pydata-sphinx-theme/blob/main/RELEASE.md) - [Commits](pydata/pydata-sphinx-theme@v0.15.4...v0.16.0) --- updated-dependencies: - dependency-name: pydata-sphinx-theme dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [rich](https://github.com/Textualize/rich) from 13.9.2 to 13.9.3. - [Release notes](https://github.com/Textualize/rich/releases) - [Changelog](https://github.com/Textualize/rich/blob/master/CHANGELOG.md) - [Commits](Textualize/rich@v13.9.2...v13.9.3) --- updated-dependencies: - dependency-name: rich dependency-type: indirect update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [safety-schemas](https://github.com/pyupio/safety_schemas) from 0.0.5 to 0.0.7. - [Commits](https://github.com/pyupio/safety_schemas/commits) --- updated-dependencies: - dependency-name: safety-schemas dependency-type: indirect update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Split the search header method to reduce complexity.
Split the search header method to reduce complexity.
…d_textlines [REF]: generate_table_bbox -> split into get_user_provided_bboxes [REF]: generate_table_bbox -> split into get_filtered_textlines
Fixup remove unused import
Bumps [safety-schemas](https://github.com/pyupio/safety_schemas) from 0.0.7 to 0.0.8. - [Commits](https://github.com/pyupio/safety_schemas/commits) --- updated-dependencies: - dependency-name: safety-schemas dependency-type: indirect update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [safety](https://github.com/pyupio/safety) from 3.2.8 to 3.2.9. - [Release notes](https://github.com/pyupio/safety/releases) - [Changelog](https://github.com/pyupio/safety/blob/main/CHANGELOG.md) - [Commits](pyupio/safety@3.2.8...3.2.9) --- updated-dependencies: - dependency-name: safety dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [pydantic](https://github.com/pydantic/pydantic) from 2.5.3 to 2.9.2. - [Release notes](https://github.com/pydantic/pydantic/releases) - [Changelog](https://github.com/pydantic/pydantic/blob/main/HISTORY.md) - [Commits](pydantic/pydantic@v2.5.3...v2.9.2) --- updated-dependencies: - dependency-name: pydantic dependency-type: indirect update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
1. **Dedicated Methods**: The code has been split into several dedicated methods: - `_set_vertical_edges`: Handles the logic for setting vertical edges. - `_update_vertical_edges`: Updates the edges for vertical lines based on the computed indices. - `_set_horizontal_edges`: Handles the logic for setting horizontal edges. - `_update_horizontal_edges`: Updates the edges for horizontal lines based on the computed indices. - `_find_close_point`: A helper method to find the closest point and has been moved outside of `set_edges` for better organization. 2. **Reduced Complexity**: Each method now has a clear purpose, which reduces the complexity of the `set_edges` method itself. This makes it easier to read and understand. 3. **Maintainability**: With the separate methods for setting vertical and horizontal edges, any changes to that logic can be made in isolation. 4. **Return Value**: The method still returns `self`, maintaining the original functionality and allowing for method chaining if desired.
Fixes #210
The condition `hspan and not left` expands to `(not left or not right) and not left` which simplifies to `not left`. After this change `Cell.hspan` and `Cell.vspan` aren't used anymore in this codebase. They stay because they are part of the API.
Collaborator
Author
|
Merged in #226 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Superseeds #53