Skip to content
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

fix flake8 config #7321

Merged
merged 3 commits into from Nov 28, 2022
Merged

fix flake8 config #7321

merged 3 commits into from Nov 28, 2022

Conversation

mathause
Copy link
Collaborator

flake8 v6 now errors on inline comments in the config file. I don't like it but oh well...

@keewis
Copy link
Collaborator

keewis commented Nov 25, 2022

some context on the change

yeah, that's a shame, especially since adding support for inline comments would be as easy as passing inline_comment_prefixes=("#",) to the RawConfigParser constructor. I suppose the fear was that with that the # character would be impossible to use in settings, which would make this a limitation of the configuration format. I think switching to a different format (yaml, toml) would have been possible, but I guess that's another can of worms.

I can understand that they're doing something, though, because silently ignoring based on letters from what is supposed to be comments is pretty bad.

To improve on this, we might be able to move the comments to a continuous block and prefix them with the error code, then use a collapsed (comma-separated) list of codes for the actual ignores. Something like this, maybe?

ignore =
    # E203: whitespace before ':' - doesn't work well with black
    # E402: module level import not at top of file
    # E501: line too long - let black worry about that
    # E731: do not assign a lambda expression, use a def
    # W50: line break before binary operator
    E203,E402,E501,E731,W50

The disadvantage would be that adding a new ignore changes two lines, and should we have lots of ignores the line wrapping would be a bit weird.

@Illviljan Illviljan added the plan to merge Final call for comments label Nov 27, 2022
@mathause
Copy link
Collaborator Author

Thanks for the context @keewis - I like your suggestion & updated the PR. AFAIK we have not changed the ignored values in a very long time. I'll merge once the tests ran through.

@mathause mathause merged commit 6f1cf51 into pydata:main Nov 28, 2022
@mathause mathause deleted the fix_flake8_config branch November 28, 2022 10:33
dcherian added a commit to dcherian/xarray that referenced this pull request Dec 2, 2022
* upstream/main: (39 commits)
  Support the new compression argument in netCDF4 > 1.6.0 (pydata#6981)
  Remove setuptools-scm-git-archive, require setuptools-scm>=7 (pydata#7253)
  Fix mypy failures (pydata#7343)
  Docs: add example of writing and reading groups to netcdf (pydata#7338)
  Reset file pointer to 0 when reading file stream (pydata#7304)
  Enable mypy warn unused ignores (pydata#7335)
  Optimize some copying (pydata#7209)
  Add parse_dims func (pydata#7051)
  Fix coordinate attr handling in `xr.where(..., keep_attrs=True)` (pydata#7229)
  Remove code used to support h5py<2.10.0 (pydata#7334)
  [pre-commit.ci] pre-commit autoupdate (pydata#7330)
  Fix PR number in what’s new (pydata#7331)
  Enable `origin` and `offset` arguments in `resample` (pydata#7284)
  fix doctests: supress urllib3 warning (pydata#7326)
  fix flake8 config (pydata#7321)
  implement Zarr v3 spec support (pydata#6475)
  Fix polyval overloads (pydata#7315)
  deprecate pynio backend (pydata#7301)
  mypy - Remove some ignored packages and modules (pydata#7319)
  Switch to T_DataArray in .coords (pydata#7285)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants