XuehaiPan / nvitop

An interactive NVIDIA-GPU process viewer and beyond, the one-stop solution for GPU process management.
https://nvitop.readthedocs.io
Apache License 2.0
4.8k stars 150 forks source link

fix(collector): fix documentation for `ResourceMetricCollector.clear()` function #132

Closed MyGodItsFull0fStars closed 3 months ago

MyGodItsFull0fStars commented 3 months ago

The docstrings of ResourceMetricCollector referred to the clear() function as reset(), which is not existing. Therefore, I replaced each mention of reset() with clear() in the documentation.

Issue Type

Runtime Environment

Description

Within the ResourceMetricCollector class, the clear() function was not used in the documentation for the class, nor the docstring of the function itself, but reset().

As the reset() function is not existing for ResourceMetricCollector, I replaced all occurrences of it in the documentation with clear() as well as changing the description of the clear() function to reflect it's name better.

Motivation and Context

The motivation is, that I was looking at the documentation and tried to use the reset() function for a ResourceMetricCollector instance, and encountered an error. I had to look at the code to see that the function I have to use is called clear().

Testing

As I only changed the docstrings within the ResourceMetricCollector class, I did not test my changes.

XuehaiPan commented 3 months ago

Hi @MyGodItsFull0fStars, thanks for raising this. Would you mind also add an alias above the method to make both name work?

class ResourceMetricCollector:
    ...

    def clear(self, tag: str | None = None) -> None:
        ...

    reset = clear

    ...
MyGodItsFull0fStars commented 3 months ago

Hi @XuehaiPan, thank you for the quick reply.

Sure, will gladly do so. I will push your suggestions once I tested both variants.

MyGodItsFull0fStars commented 3 months ago

@XuehaiPan I just pushed the commit that includes the reset alias for the clear function in ResourceMetricCollector.

MyGodItsFull0fStars commented 3 months ago

Hey @XuehaiPan, here is a friendly reminder that I have added the requested alias for reset and the code should now be fine.