Skip to content

Unvendor cpuinfo#373

Merged
jakirkham merged 2 commits intozarr-developers:mainfrom
DimitriPapadopoulos:cpuinfo
Oct 29, 2022
Merged

Unvendor cpuinfo#373
jakirkham merged 2 commits intozarr-developers:mainfrom
DimitriPapadopoulos:cpuinfo

Conversation

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Oct 28, 2022

Because the cpuinfo module is required by setup.py, we need to pull it with pyproject.toml.

I think it is not needed in requirements_dev.txt.

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)

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 28, 2022

This pull request fixes 5 alerts when merging 67dec4d into d12b54b - view on LGTM.com

fixed alerts:

  • 4 for Unused local variable
  • 1 for Module is imported with 'import' and 'import from'

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.

On PyPI this is called py-cpuinfo, so suggesting updating requirements below accordingly

@jakirkham
Copy link
Copy Markdown
Member

Thanks Dimitri! 🙏

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 28, 2022

This pull request fixes 5 alerts when merging 19ea966 into d12b54b - view on LGTM.com

fixed alerts:

  • 4 for Unused local variable
  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 28, 2022

This pull request fixes 5 alerts when merging 8dc047f into d12b54b - view on LGTM.com

fixed alerts:

  • 4 for Unused local variable
  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 28, 2022

This pull request fixes 5 alerts when merging 7059ebc into 0a2ec9f - view on LGTM.com

fixed alerts:

  • 4 for Unused local variable
  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 28, 2022

This pull request fixes 5 alerts when merging 8705ee3 into 0a2ec9f - view on LGTM.com

fixed alerts:

  • 4 for Unused local variable
  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 28, 2022

This pull request fixes 5 alerts when merging c649c9b into 0a2ec9f - view on LGTM.com

fixed alerts:

  • 4 for Unused local variable
  • 1 for Module is imported with 'import' and 'import from'

Bacause the cpuinfo module is required by setup.py, we need to
install it with pyproject.toml.
@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review October 29, 2022 08:01
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 29, 2022

This pull request fixes 5 alerts when merging 6164e68 into 0a2ec9f - view on LGTM.com

fixed alerts:

  • 4 for Unused local variable
  • 1 for Module is imported with 'import' and 'import from'

@jakirkham jakirkham enabled auto-merge (squash) October 29, 2022 20:37
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.

Looks good. Thanks Dimitri! 🙏

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 29, 2022

This pull request fixes 5 alerts when merging 1e1d58e into 61a90da - view on LGTM.com

fixed alerts:

  • 4 for Unused local variable
  • 1 for Module is imported with 'import' and 'import from'

@jakirkham jakirkham merged commit 7d11776 into zarr-developers:main Oct 29, 2022
@DimitriPapadopoulos DimitriPapadopoulos deleted the cpuinfo branch October 29, 2022 22:24
@joshmoore
Copy link
Copy Markdown
Member

Thanking of the upcoming release, best to bump to 0.11 with the new dependency?

@jakirkham
Copy link
Copy Markdown
Member

Agreed. Opened issue ( #377 ) so we can sort out anything for the next release. Hoping to have a couple more build fixes done, but will see. Depends on time constraints & help from others (like with pyproject.toml)

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