3 ms·

I want some feedback on my new Python library

1 points by traman124 3y ago

traman124 3y ago
Hi everyone! I would like some feedback on my new Python library, PlatformInfo. It's still WIP and I would really appreciate feedback and suggestions for it. Thanks so much!
eesmith 3y ago
Try using "AskHN: " or "ShowHN: " to get more appropriate traction. Use a more mainstream Python naming scheme. I expect "Parser" to be a class name, not a function name. Most Python code uses underscore_names rather than camelCase. Use subprocess.run() or subprocess.check_output() instead of writing your own variant using a pipe. You are supposed to wait on the process before closing, though garbage collection does that for you here. Avoid the extra overhead of "shell=True" since you can just write ["prog", "opt"] or "prog opt".split(). Consider using subclasses like MacPlatform, LinuxPlatform, and BSDPlatform so you don't need the "if self.platform" in each method. This would also make your methods smaller and more focused. A downside is it's harder to re-use implementations for different classes. I generally use a mixin for that, for example, "_uname_m" might implement arch() for those OSes, class _uname_m: def arch(self): return subprocess.run(["uname", "-m"]) class MacPlatform(_uname_m, Platform): ... You may consider hard-coding path names like /usr/bin/arch for the OS, to prevent path issues. This: if arches[arch] != arch: return arches[arch] return arch can be replaced with arches.get(arch, arch) A related package is https://github.com/workhorsy/py-cpuinfo https://github.com/workhorsy/py-cpuinfo . You might look at it to see how they organize platform differences.
- traman124 3y ago
  Thanks so much! My code quality system has been sending me emails every time I commit because of that shell=true thing, I think it's about time to change it. I'll switch to a better naming scheme if that makes my code more standard or easier to understand :D I also like your input on the architecture functions, although my Windows implementation is really bad and I know it (it's very alpha quality) (EDIT: That library you mentioned has some good ideas on fetching cpu info on Windows, whic has been a bit of a problem for me to implement) (EDIT 2: I have started to fix these issues)
H4K3R13 3y ago
Good work, what makes it different from other libraries like " os "
- traman124 3y ago
  The os module is less for fetching system info and more for performing system level actions.PlatformInfo is more similar to the platform library (which is part of the python stdlib) but my intention is for it to be more granular than the inbuilt platform library
JoeyBananas 3y ago
I don't hate it.
- traman124 3y ago
  ok??? :D
traman124 3y ago
I am starting to add CPU detection support to PlatformInfo. A partial implementation for inux is planned for beta 4, full implementation for all OSes beta 5.

1 points by traman124 3y ago

Hi everyone! I would like some feedback on my new Python library, PlatformInfo. It's still WIP and I would really appreciate feedback and suggestions for it. Thanks so much!

eesmith 3y ago

Try using "AskHN: " or "ShowHN: " to get more appropriate traction. Use a more mainstream Python naming scheme. I expect "Parser" to be a class name, not a function name. Most Python code uses underscore_names rather than camelCase. Use subprocess.run() or subprocess.check_output() instead of writing your own variant using a pipe. You are supposed to wait on the process before closing, though garbage collection does that for you here. Avoid the extra overhead of "shell=True" since you can just write ["prog", "opt"] or "prog opt".split(). Consider using subclasses like MacPlatform, LinuxPlatform, and BSDPlatform so you don't need the "if self.platform" in each method. This would also make your methods smaller and more focused. A downside is it's harder to re-use implementations for different classes. I generally use a mixin for that, for example, "_uname_m" might implement arch() for those OSes, class _uname_m: def arch(self): return subprocess.run(["uname", "-m"]) class MacPlatform(_uname_m, Platform): ... You may consider hard-coding path names like /usr/bin/arch for the OS, to prevent path issues. This: if arches[arch] != arch: return arches[arch] return arch can be replaced with arches.get(arch, arch) A related package is https://github.com/workhorsy/py-cpuinfo https://github.com/workhorsy/py-cpuinfo . You might look at it to see how they organize platform differences.

traman124 3y ago
Thanks so much! My code quality system has been sending me emails every time I commit because of that shell=true thing, I think it's about time to change it. I'll switch to a better naming scheme if that makes my code more standard or easier to understand :D I also like your input on the architecture functions, although my Windows implementation is really bad and I know it (it's very alpha quality) (EDIT: That library you mentioned has some good ideas on fetching cpu info on Windows, whic has been a bit of a problem for me to implement) (EDIT 2: I have started to fix these issues)

H4K3R13 3y ago

Good work, what makes it different from other libraries like " os "

traman124 3y ago
The os module is less for fetching system info and more for performing system level actions.PlatformInfo is more similar to the platform library (which is part of the python stdlib) but my intention is for it to be more granular than the inbuilt platform library

JoeyBananas 3y ago

I don't hate it.

traman124 3y ago
ok??? :D

traman124 3y ago

I am starting to add CPU detection support to PlatformInfo. A partial implementation for inux is planned for beta 4, full implementation for all OSes beta 5.