Ensure pure-Python implementations **do not** invoke code from the binary extension. #160
Conversation
This file contains 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
The goal is to replace the original names with a wrapper that **only** exports the parts needed elsewhere and put the pure Python implementations by themselves. Done via: ``` git mv _curve_helpers.py _py_curve_helpers.py git mv _geometric_intersection.py _py_geometric_intersection.py git mv _helpers.py _py_helpers.py git mv _intersection_helpers.py _py_intersection_helpers.py git mv _surface_helpers.py _py_surface_helpers.py git mv _surface_intersection.py _py_surface_intersection.py ``` Found these modules via: ``` git grep -l 'if _speedup is None' ```
These are intended to stand in for the previous modules, and the rename is intentional (and temporary) to suss out places where the imports are relying on pure Python parts vs. parts that could either by Python or speedup.
Found via `nox -s lint`.
Done via: ``` git mv test__curve_helpers.py test_py_curve_helpers.py git mv test__geometric_intersection.py test_py_geometric_intersection.py git mv test__helpers.py test_py_helpers.py git mv test__intersection_helpers.py test_py_intersection_helpers.py git mv test__surface_helpers.py test_py_surface_helpers.py git mv test__surface_intersection.py test_py_surface_intersection.py ``` No code edits were made in this commit, but some will need to be done to fix imports and split out the "wrap" portions.
Done via: ``` cd test/unit/ git mv test_py_curve_helpers.py test__py_curve_helpers.py git mv test_py_geometric_intersection.py test__py_geometric_intersection.py git mv test_py_helpers.py test__py_helpers.py git mv test_py_intersection_helpers.py test__py_intersection_helpers.py git mv test_py_surface_helpers.py test__py_surface_helpers.py git mv test_py_surface_intersection.py test__py_surface_intersection.py ```
In the process had to fix a runtime import of a Python module within `_speedup.pyx`.
This was for functions that also occurred as `_speedup.{name}` that
have now been renamed (from `_{name}`) to `{name}`.
Also cleaned up the `_{name}` references in documentation wherever
I could find references.
This was only temporary to ensure all the old names had been properly moved. Done via: ``` git mv src/python/bezier/_wrap_curve_helpers.py src/python/bezier/_curve_helpers.py git mv src/python/bezier/_wrap_geometric_intersection.py src/python/bezier/_geometric_intersection.py git mv src/python/bezier/_wrap_helpers.py src/python/bezier/_helpers.py git mv src/python/bezier/_wrap_intersection_helpers.py src/python/bezier/_intersection_helpers.py git mv src/python/bezier/_wrap_surface_helpers.py src/python/bezier/_surface_helpers.py git mv src/python/bezier/_wrap_surface_intersection.py src/python/bezier/_surface_intersection.py git mv tests/unit/test__wrap_curve_helpers.py tests/unit/test__curve_helpers.py git mv tests/unit/test__wrap_geometric_intersection.py tests/unit/test__geometric_intersection.py git mv tests/unit/test__wrap_helpers.py tests/unit/test__helpers.py git mv tests/unit/test__wrap_intersection_helpers.py tests/unit/test__intersection_helpers.py git mv tests/unit/test__wrap_surface_helpers.py tests/unit/test__surface_helpers.py git mv tests/unit/test__wrap_surface_intersection.py tests/unit/test__surface_intersection.py ``` A subsequent commit will update the **references** to these files.
Done via: ``` git grep -l _wrap -- ':(exclude)*.c' | xargs sed -i s/_wrap/''/g ``` Still need to follow-up to: - Resolve lint errors due to alphabetical ordering of imports - Re-generate `*.c` files with Cython
Slipped in some changes where `black` does not have an "opinion". In particular, tried to make sure there is an empty line after the LICENSE header and make sure there are 2 empty lines between imports and constants.
- Removing the `HAS_SPEEDUP` check since the tests for the pure Python implementations now eschew usage of speedups (even if present) - Explicitly re-defining the marked methods in the subclass to avoid marking them as slow
Pylint (correctly) flagged them as `useless-super-delegation` and so I disabled the rule for the few methods where I re-defined methods to drop the `slow` mark in pytest.
This was referenced Jan 6, 2020
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.
Fixes #159.
This was way more work than I thought it would be, but I think long-term it will be worth it.