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(collector): fix documentation for ResourceMetricCollector.clear() function #132

Merged
merged 5 commits into from
Aug 7, 2024

Conversation

MyGodItsFull0fStars
Copy link
Contributor

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

  • Improvement/feature implementation

Runtime Environment

  • Operating system and version: [e.g. Ubuntu 20.04 LTS / Windows 10 Build 19043.1110]
  • Terminal emulator and version: [e.g. GNOME Terminal 3.36.2 / Windows Terminal 1.8.1521.0]
  • Python version: [e.g. 3.7.2 / 3.9.6]
  • NVML version (driver version): [e.g. 460.84]
  • nvitop version or commit: [e.g. 0.10.0 / 0.10.1.dev7+ga083321 / main@75ae3c]
  • python-ml-py version: [e.g. 11.450.51]
  • Locale: [e.g. C / C.UTF-8 / en_US.UTF-8]

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 XuehaiPan added bug Something isn't working api Something related to the core APIs labels Jul 22, 2024
@XuehaiPan
Copy link
Owner

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
Copy link
Contributor Author

Hi @XuehaiPan, thank you for the quick reply.

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

@MyGodItsFull0fStars
Copy link
Contributor Author

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

@MyGodItsFull0fStars
Copy link
Contributor Author

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

@XuehaiPan XuehaiPan self-assigned this Aug 7, 2024
@XuehaiPan XuehaiPan added the documentation Improvements or additions to documentation label Aug 7, 2024
@XuehaiPan XuehaiPan changed the title Fixed documentation for ResourceMetricCollector.clear() function fix(collector): fix documentation for ResourceMetricCollector.clear() function Aug 7, 2024
@XuehaiPan XuehaiPan merged commit 80100c7 into XuehaiPan:main Aug 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Something related to the core APIs bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants