Action ruff checks - #416
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Templating
This PR includes changes that may be worth sharing via templating. For each file listed below, please either:
- Action the suggestion via a pull request editing/adding the relevant file in the SciTools/.github templates/ directory. 1
- Raise an issue against the SciTools/.github repo for the above action if you really don't have 10mins spare right now. Include an assignee, to avoid it being forgotten.
- Dismiss the suggestion if the changes are not suitable for templating.
You will need to dismiss this review before this PR can be merged. Recommend the reviewer does this as their final action before merging, as this text will continually update as commits come in.
Templated files
The following changed files are templated:
- pyproject.toml, templated by SciTools/.github/templates/pyproject.toml
Template candidates
The following changed files are not currently templated, but their parent directories suggest they may be good candidates for a new template to be created:
Footnotes
-
Include this text in the PR body to avoid any notifications about applying the template changes back to the source repo!
@scitools-templating: please no update notification on: python-stratify ↩
Sorry, something went wrong.
trexfeathers
left a comment
There was a problem hiding this comment.
Thanks @HGWright, some requests for you
Sorry, something went wrong.
| output_array[:] = np.inf if direction > 0 else -np.inf | ||
|
|
||
|
|
||
| class TestColumnInterpolation(unittest.TestCase): |
There was a problem hiding this comment.
Why have you done this in some places but not in others? test_bounded_vinterp.py still has some examples.
Sorry, something went wrong.
| vdims = list(set(range(z_src.ndim)) - set([axis_relative])) | ||
| z_src_reshaped = np.transpose(z_src, [axis_relative] + vdims) | ||
| z_target_reshaped = np.transpose(z_target, [axis_relative] + vdims) | ||
| # vdims = list(set(range(z_src.ndim)) - set([axis_relative])) |
There was a problem hiding this comment.
Commented code
Sorry, something went wrong.
| invert_transpose = [data_transpose.index(ind) for ind in list(range(result.ndim))] | ||
| result = result.transpose(invert_transpose) | ||
| return result | ||
| # result = result.transpose(invert_transpose) |
There was a problem hiding this comment.
Commented code
Sorry, something went wrong.
There was a problem hiding this comment.
It's cool that Ruff works on notebooks. It's also cool the improved pattern it found for randomness. 👍
Sorry, something went wrong.
| "T201", # print found | ||
|
|
||
| ] | ||
| "S101", # Use of assert detected. |
There was a problem hiding this comment.
You needed to add this one, presumably because you've been converting the tests to use PyTest? If you, could you move this exception down to the file-specific exceptions for src/stratify/tests/*.py?
Sorry, something went wrong.
|
|
||
|
|
||
| def src_data(shape=(400, 500, 100), lazy=False): | ||
| def src_data(shape=(400, 500, 100)): |
There was a problem hiding this comment.
I'm not quite sure of the design of this module, but lazy is very much still used. Seems like Ruff made a mistake?
python-stratify/src/stratify/tests/performance.py
Lines 44 to 45 in 612de10
python-stratify/src/stratify/tests/performance.py
Lines 18 to 24 in 612de10
Sorry, something went wrong.
There was a problem hiding this comment.
This file has a lot more unittest left in it than test_vinterp does.
- Still inheriting from unittest.TestCase
- Still calling unittest.main() at the bottom
- Maybe other stuff I've missed?
I'm guessing you went this way because the tests in this file include setUp() methods, which are a unittest thing so the tests cannot run without unittest?
Anyway it's confusing to have these different states; I would prefer everything or nothing; options:
- Reverse the appropriate changes in test_vinterp
OR - Run through the PyTest conversion checklist, which we have kept up specifically to help with the other repos beyond Iris: https://scitools-iris.readthedocs.io/en/latest/developers_guide/contributing_pytest_conversions.html
Sorry, something went wrong.
Reviewers
Requested changes must be addressed to merge this pull request.
🚀 Pull Request
Description
From #277. We have been tracking the ignored checks for Ruff. This PR goes through and actions all of the applicable checks and ignores the ones we cant deal with yet, mainly around type hinting and documentation.
Part of these checks is moving from references to unittest to pytest, and from assertEqual to just regular assert statements