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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace flakeheaven and isort with ruff #2747
Replace flakeheaven and isort with ruff #2747
Changes from 4 commits
451cb61ece073003e0357a208ca8b369811ead3abeFile filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously with
flakeheaven, we setmax_line_length=88(to matchblack). But settingline-length=88withruffwithout settingignore = ["E501"]would raise errors on lines like these:Note that
blackdoesn't format these code lines over 88 characters, butruffcomplains about it. The thing is, we recently disabledpylint's line-length checking in #2735, because we assumed that flakeheaven's pycodestyle checker would handle it, but ruff's pycodestyle checker is a little different.So what should we do in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we ignore both
E501(for codes) andW505(for docs) inruffand enablepylint's line-length checking (I assume it checks both codes and docs).For long URLs, we can either add
# pylint: disable=line-too-longto the lines or improve the regex for theignore-long-linesoption to make it work for lines likeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm considering that we replace
pylintwithruffeventually (see #2741 (comment)), so would prefer not to addpylint's line-length checking back.E501 is ignored already right now. We could ignore
W505too I suppose, assuming thatblackdocanddocformatteris sufficient, and we don't plan to replace both withruff🙂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docformatter: astral-sh/ruff#1335blackdoc: astral-sh/ruff#7146There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm actually, I just did some tesitng and maybe we shouldn't ignore
W505. I just tried ignoringW505in pyproject.toml and removed all thenoqa: W505, and blackdoc/docformatter doesn't format things properly when I runmake format:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.astral.sh/ruff/rules/doc-line-too-long/#why-is-this-bad
I think it means that we can simply remove all
noqa: W505and don't have to ignoreW505.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried removing all the
noqa: W505in a208ca8. Style Checks show these errors:ruff check pygmt doc/conf.py examples pygmt/datasets/earth_age.py:75:80: W505 Doc line too long (133 > 79 characters) | 73 | (i.e., ``grid.gmt.registration`` and ``grid.gmt.gtype`` respectively). 74 | However, these properties may be lost after specific grid operations (such 75 | as slicing) and will need to be manually set before passing the grid to any PyGMT data processing or plotting functions. Refer to | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ W505 76 | :class:`pygmt.GMTDataArrayAccessor` for detailed explanations and 77 | workarounds. | pygmt/datasets/samples.py:334:80: W505 Doc line too long (80 > 79 characters) | 332 | >>> # use list_sample_data to see the available datasets 333 | >>> pprint(list_sample_data(), width=120) 334 | {'bathymetry': 'Table of ship bathymetric observations off Baja California', | ^ W505 335 | 'earth_relief_holes': 'Regional 20 arc-minutes Earth relief grid with holes', 336 | 'fractures': 'Table of hypothetical fracture lengths and azimuths', | pygmt/datasets/samples.py:335:80: W505 Doc line too long (82 > 79 characters) | 333 | >>> pprint(list_sample_data(), width=120) 334 | {'bathymetry': 'Table of ship bathymetric observations off Baja California', 335 | 'earth_relief_holes': 'Regional 20 arc-minutes Earth relief grid with holes', | ^^^ W505 336 | 'fractures': 'Table of hypothetical fracture lengths and azimuths', 337 | 'hotspots': 'Table of locations, names, and symbol sizes of hotpots from Müller et al. (1993)', | pygmt/datasets/samples.py:337:80: W505 Doc line too long (100 > 79 characters) | 335 | 'earth_relief_holes': 'Regional 20 arc-minutes Earth relief grid with holes', 336 | 'fractures': 'Table of hypothetical fracture lengths and azimuths', 337 | 'hotspots': 'Table of locations, names, and symbol sizes of hotpots from Müller et al. (1993)', | ^^^^^^^^^^^^^^^^^^^^^ W505 338 | 'japan_quakes': 'Table of earthquakes around Japan from the NOAA NGDC database', 339 | 'mars_shape': 'Table of topographic signature of the hemispheric dichotomy of Mars from Smith and Zuber (1996)', | pygmt/datasets/samples.py:338:80: W505 Doc line too long (85 > 79 characters) | 336 | 'fractures': 'Table of hypothetical fracture lengths and azimuths', 337 | 'hotspots': 'Table of locations, names, and symbol sizes of hotpots from Müller et al. (1993)', 338 | 'japan_quakes': 'Table of earthquakes around Japan from the NOAA NGDC database', | ^^^^^^ W505 make: *** [Makefile:72: check] Error 1 339 | 'mars_shape': 'Table of topographic signature of the hemispheric dichotomy of Mars from Smith and Zuber (1996)', 340 | 'maunaloa_co2': 'Table of CO2 readings from Mauna Loa', | pygmt/datasets/samples.py:339:80: W505 Doc line too long (117 > 79 characters) | 337 | 'hotspots': 'Table of locations, names, and symbol sizes of hotpots from Müller et al. (1993)', 338 | 'japan_quakes': 'Table of earthquakes around Japan from the NOAA NGDC database', 339 | 'mars_shape': 'Table of topographic signature of the hemispheric dichotomy of Mars from Smith and Zuber (1996)', | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ W505 340 | 'maunaloa_co2': 'Table of CO2 readings from Mauna Loa', 341 | 'notre_dame_topography': 'Table 5.11 in Davis: Statistics and Data Analysis in Geology', | pygmt/datasets/samples.py:341:80: W505 Doc line too long (93 > 79 characters) | 339 | 'mars_shape': 'Table of topographic signature of the hemispheric dichotomy of Mars from Smith and Zuber (1996)', 340 | 'maunaloa_co2': 'Table of CO2 readings from Mauna Loa', 341 | 'notre_dame_topography': 'Table 5.11 in Davis: Statistics and Data Analysis in Geology', | ^^^^^^^^^^^^^^ W505 342 | 'ocean_ridge_points': 'Table of ocean ridge points for the entire world', 343 | 'rock_compositions': 'Table of rock sample compositions', | pygmt/src/nearneighbor.py:43:80: W505 Doc line too long (80 > 79 characters) | 41 | Grid table data using a "Nearest neighbor" algorithm. 42 | 43 | **nearneighbor** reads arbitrarily located (*x*, *y*, *z*\ [, *w*]) triplets | ^ W505 44 | [quadruplets] and uses a nearest neighbor algorithm to assign a weighted 45 | average value to each node that has one or more data points within a search | pygmt/src/nearneighbor.py:142:80: W505 Doc line too long (83 > 79 characters) | 140 | >>> data = pygmt.datasets.load_sample_data(name="bathymetry") 141 | >>> # Create a new grid with 5 arc-minutes spacing in the designated region 142 | >>> # Set search_radius to only consider points within 10 arc-minutes of a node | ^^^^ W505 143 | >>> output = pygmt.nearneighbor( 144 | ... data=data, | Found 9 errors.You're right that the long URLs from #2728 don't raise W505 errors anymore, but there are 9 others we'll need to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that
ruff'sisortrules is nearly-equivalent toisort'sprofile="black"settings (see https://docs.astral.sh/ruff/faq/#how-does-ruffs-import-sorting-compare-to-isort). Also, files that are gitignored are automatically skipped from being check (see astral-sh/ruff#1234), so no need to apply these rules anymore.