8000 Refactor/Chore: Remove access parameters in client by j279li · Pull Request #290 · polaris-hub/polaris · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Refactor/Chore: Remove access parameters in client #290

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

j279li
Copy link
@j279li j279li commented May 13, 2025

This PR removes the AccessType type, simplifying the API by eliminating the access parameter, which was previously used to specify public or private access when uploading artifacts to the Polaris Hub.

Removal of AccessType and access parameter:

  • Type and Imports Cleanup:

    • Removed AccessType from imports in multiple files, including polaris/benchmark/_base.py, polaris/dataset/_base.py, polaris/dataset/_dataset.py, polaris/dataset/_dataset_v2.py, polaris/evaluate/_results.py, polaris/hub/client.py, and polaris/model/__init__.py.
  • Parameter Removal in Method Definitions:

    • Removed the access parameter from method signatures such as upload_to_hub in various modules (polaris/benchmark/_base.py, < 8000 code class="notranslate">polaris/dataset/_base.py, polaris/dataset/_dataset.py, polaris/dataset/_dataset_v2.py, polaris/evaluate/_results.py, polaris/hub/client.py, and polaris/model/__init__.py).
  • Documentation Updates:

    • Adjusted method docstrings to remove mentions of the access parameter, ensuring the documentation aligns with the updated API.

    Notion Package: https://www.notion.so/Private-Artifact-Removal-1ec0c0d8de7080cf9b6fdde6b7fc0dd8

@j279li j279li self-assigned this May 13, 2025
@j279li j279li requested a review from cwognum as a code owner May 13, 2025 21:07
@j279li j279li added the chore label May 13, 2025
Copy link
Contributor
@jstlaurent jstlaurent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Some of these methods will be removed by @danielpeng1's #289 , but the change to model upload is still needed.

@j279li
Copy link
Author
j279li commented May 14, 2025

👍 Some of these methods will be removed by @danielpeng1's #289 , but the change to model upload is still needed.

Yeah I'll wait for his PR to be merged, and adjust as needed. Or I could merge this first.

Copy link
Contributor
@Andrewq11 Andrewq11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @j279li! Left one minor comment in addition to echoing Julien's.

response = self._base_request_to_hub(
url="/v2/result", method="POST", json={"access": access, **result_json}
)
response = self._base_request_to_hub(url="/v2/result", method="POST", json={**result_json})
Copy link
Contributor
@Andrewq11 Andrewq11 May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:

Suggested change
response = self._base_request_to_hub(url="/v2/result", method="POST", json={**result_json})
response = self._base_request_to_hub(url="/v2/result", method="POST", json=result_json)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0