-
Notifications
You must be signed in to change notification settings - Fork 284
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
Introduce gpustat package #33
Conversation
Can't agree more. The current main file has too many lines, and the single file executing makes little sense as dependencies are introduced in previous PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think gpustat/gpustat.py
is not clear enough, i recommend to use several separate files for clarification.
gpustat/__init__.py
Outdated
|
||
__version__ = '0.5.0.dev1' | ||
|
||
from .gpustat import GPUStat, GPUStatCollection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to put these classes in separate file e.g. gpu_stat_collection.py
and gpu_stat.py
.
So as the new_query
and print_gpustat
in utils.py
And I think the gpustat.gpustat
is a little frustrating.
Naming conventions are to be discussed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gpustat.gpustat
sounds funny, but I have no better naming. Having two files separately for those classes sounds too excessive, but agree with having print-related stuffs in a separate one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you have a package, you don't need the gpustat.py
. A suggestion of file structure:
- gpu_stat_collection.py: classes
GPUStat
andGPUStatCollection
- display.py output.py or utils.py : the print-related stuffs (There are many lines related to that)
- main.py: the main function (currently
.gpustat.main
)
A reference: https://github.com/nicolargo/glances/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it is too verbose, and actually I am looking for something simple that can nicely embrace both classes.
06d88a9
to
809e033
Compare
So we have |
809e033
to
54e731e
Compare
This is now on master. |
I plan to switch to using a package, say
gpustat
, instead of writing everything in a single filegpustat.py
.As a lot of features are going to to added, I feel the necessity of doing this. In this way, we can also add (optional) features such as monitoring, web support, etc.
@Stonesjtu How do you think of this?