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 handling of numexpr.cpu in print_versions() #495

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pinotree
Copy link

There are a couple of issues in the way numexpr.cpu.info is used in print_versions():

  1. all the subclasses of CPUInfoBase have a class attribute info, whereas CPUInfoBase does not; this means that, in case there is no CPU implementation for an OS, self.info returns a function (because of CPUInfoBase.__getattr__()) which is not subscriptable
  2. only few subclasses of CPUInfoBase (e.g. LinuxCPUInfo, Win32CPUInfo) implement info as list of dicts for each CPU, whereas the rest of the implementations use a dict with attributes

Since info seems to be mostly an internal detail, then adapt print_versions() to be a little more flexible:

  • do not try to query attributes in case the actual CPU implementation is CPUInfoBase: it is not implemented for the current OS, so there is nothing to query anyway (fixes (1))
  • get the first element of info only in case it is a list (fixes (2))
  • drop the catch of KeyError, which should not happen anymore now

There are a couple of issues in the way "numexpr.cpu.info" is used in
print_versions():

1) all the subclasses of CPUInfoBase have a class attribute "info",
   whereas CPUInfoBase does not; this means that, in case there is no
   CPU implementation for an OS, "self.info" returns a function
   (because of CPUInfoBase.__getattr__()) which is not subscriptable
2) only few subclasses of CPUInfoBase (e.g. LinuxCPUInfo, Win32CPUInfo)
   implement "info" as list of dicts for each CPU, whereas the rest of
   the implementations use a dict with attributes

Since "info" seems to be mostly an internal detail, then adapt
print_versions() to be a little more flexible:
- do not try to query attributes in case the actual CPU implementation
  is CPUInfoBase: it is not implemented for the current OS, so there is
  nothing to query anyway (fixes (1))
- get the first element of "info" only in case it is a list (fixes (2))
- drop the catch of KeyError, which should not happen anymore now
@pinotree
Copy link
Author

@FrancescAlted would you please take a look at this? thanks in advance!

@FrancescAlted
Copy link
Contributor

Can you tell us what advantages that brings for users?

Copy link

Message to comment on stale PRs. If none provided, will not mark PRs stale

@github-actions github-actions bot added the Stale label Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants