Skip to content

Conversation

@jakirkham
Copy link
Member

This was accidentally dropped when removing tox ( #1219 ). Here we add a .flake8 configuration file with those options. Note it is not currently possible to use pyproject.toml for Flake8 ( PyCQA/flake8#234 ).

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@jakirkham jakirkham mentioned this pull request Nov 3, 2022
6 tasks
@jakirkham
Copy link
Member Author

@Saransh-cpp, could you please review? 🙂

@joshmoore
Copy link
Member

Humorous (?) sidenote: https://pypi.org/project/Flake8-pyproject/

@jakirkham
Copy link
Member Author

Yeah saw that. For now think it is ok to keep this in a separate config file.

Copy link
Contributor

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @jakirkham!

flake8's config is currently stored in pre-commit-config.yaml, and the contributing docs don't instruct developers to use flake8 outside of pre-commit. There shouldn't be any issues with having it in 2 places though (similar to the codespell configuration)! 🚀

@jakirkham
Copy link
Member Author

jakirkham commented Nov 6, 2022

Ah ok. Thanks Saransh! 🙏

Will flake8 read the config from pre-commit-config.yaml? Or if not, is there a way for us to point pre-commit at .flake8?

@codecov
Copy link

codecov bot commented Nov 6, 2022

Codecov Report

Merging #1249 (c43ddcf) into main (bbc66df) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1249   +/-   ##
=======================================
  Coverage   99.99%   99.99%           
=======================================
  Files          35       35           
  Lines       14126    14127    +1     
=======================================
+ Hits        14125    14126    +1     
  Misses          1        1           
Impacted Files Coverage Δ
zarr/util.py 100.00% <0.00%> (ø)

@Saransh-cpp
Copy link
Contributor

Will flake8 read the config from pre-commit-config.yaml? Or if not, is there a way for us to point pre-commit at .flake8?

Unfortunately, no and no 🥲

pre-commit.ci also has some problems with reading configurations from files other than pre-commit-config.yaml. I guess having this at two places is the only viable option right now.

@joshmoore
Copy link
Member

Maybe let's minimally document those lines that require synchronization (and we'll keep looking for unified solutions moving forward)?

@jakirkham
Copy link
Member Author

Gotcha thanks for looking into this Saransh! 🙏

Yeah would suggest raising a new community issue about using pre-commit. Maybe others have thoughts on this too 🙂

Let's go ahead and merge this change for flake8 so we have it.

@joshmoore joshmoore merged commit 810ec5a into zarr-developers:main Nov 10, 2022
@jakirkham jakirkham deleted the restore_flake8 branch November 10, 2022 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants