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

Introduce gpustat package #33

Merged
merged 2 commits into from
Dec 1, 2017
Merged

Introduce gpustat package #33

merged 2 commits into from
Dec 1, 2017

Conversation

wookayin
Copy link
Owner

I plan to switch to using a package, say gpustat, instead of writing everything in a single file gpustat.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?

@Stonesjtu
Copy link
Collaborator

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.

Copy link
Collaborator

@Stonesjtu Stonesjtu left a 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.


__version__ = '0.5.0.dev1'

from .gpustat import GPUStat, GPUStatCollection
Copy link
Collaborator

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.

Copy link
Owner Author

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.

Copy link
Collaborator

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 and GPUStatCollection
  • 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/

Copy link
Owner Author

@wookayin wookayin Nov 25, 2017

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.

@wookayin
Copy link
Owner Author

So we have core.py that implements the two classes, and __main__.py for some main-related stuffs (so that one can run via gpustat -m as well). Will merge soon.

@wookayin wookayin merged commit 7e57f9c into master Dec 1, 2017
@wookayin
Copy link
Owner Author

wookayin commented Dec 1, 2017

This is now on master.

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