Skip to content

Apply refurb suggestions#372

Merged
jakirkham merged 1 commit intozarr-developers:mainfrom
DimitriPapadopoulos:refurb
Oct 30, 2022
Merged

Apply refurb suggestions#372
jakirkham merged 1 commit intozarr-developers:mainfrom
DimitriPapadopoulos:refurb

Conversation

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Oct 28, 2022

  • [FURB107] Replace try: ... except ImportError: pass with with suppress(ImportError): ...
  • [FURB109] Replace in [x, y, z] with in (x, y, z)
  • [FURB113] Use x.extend(...) instead of repeatedly calling x.append()
  • [FURB123] Replace list(x) with x.copy()

Not fixing all of them: some are debatable and other result in less readable (by me) code.

https://github.com/dosisod/refurb

TODO:

  • Unit tests and/or doctests in docstrings
  • tox -e py310 passes locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • tox -e docs passes locally
  • GitHub Actions CI passes
  • Test coverage to 100% (Coveralls passes)

@DimitriPapadopoulos DimitriPapadopoulos changed the title Fix refurb suggestions Apply refurb suggestions Oct 28, 2022
@DimitriPapadopoulos DimitriPapadopoulos force-pushed the refurb branch 2 times, most recently from b08df1c to 7b91ad9 Compare October 29, 2022 08:05
@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review October 29, 2022 08:06
@jakirkham jakirkham enabled auto-merge (squash) October 29, 2022 21:26
Copy link
Copy Markdown
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Thanks Dimitri! 🙏

Merging after CI passes

* [FURB107]
  Replace `try: ... except ImportError: pass` with `with suppress(ImportError): ...`
* [FURB109]
  Replace `in [x, y, z]` with `in (x, y, z)`
* [FURB113]
  Use `x.extend(...)` instead of repeatedly calling `x.append()`
* [FURB123]
  Replace `list(x)` with `x.copy()`
auto-merge was automatically disabled October 29, 2022 22:29

Head branch was pushed to by a user without write access

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor Author

Rebased because of conflicts.

@jakirkham jakirkham merged commit e929c96 into zarr-developers:main Oct 30, 2022
@jakirkham
Copy link
Copy Markdown
Member

Thanks Dimitri! It's in 🙂

@DimitriPapadopoulos DimitriPapadopoulos deleted the refurb branch October 30, 2022 00:20
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.

2 participants