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

Add more type annotations in PKB #5236

Open
jellyfishcake opened this issue Oct 4, 2024 · 7 comments
Open

Add more type annotations in PKB #5236

jellyfishcake opened this issue Oct 4, 2024 · 7 comments
Assignees
Labels
good first issue This is a good issue for someone new to PKB

Comments

@jellyfishcake
Copy link
Contributor

Add more type annotations to almost any file in PKB. Python is by default untyped, but optional type annotations add clarity & help detect errors.

Some files like https://github.com/GoogleCloudPlatform/PerfKitBenchmarker/blob/master/perfkitbenchmarker/providers/gcp/vertex_ai.py already have type annotations. Many have some annotations but are also missing some.

See https://docs.python.org/3/library/typing.html for more information & examples of type annotations.

After adding type annotations, use pytype to check if annotations are correct or break anything. See here for install instructions:

https://github.com/google/pytype

But the short version is

pip install pytype
pytype _file.py

will output if the file has any errors after adding the type annotations

Some classes could also use attribute lists like:

class VertexAiModelSpec(managed_ai_model_spec.BaseManagedAiModelSpec):
"""Spec for a Vertex AI model.

Attributes:
env_vars: Environment variables set on the node.
serving_container_command: Command run on container to start the model.
serving_container_args: The arguments passed to container create.
"""

def init():
self.container_image_uri: str
self.model_bucket_suffix: str
self.serving_container_command: list[str]

@jellyfishcake jellyfishcake added the good first issue This is a good issue for someone new to PKB label Oct 4, 2024
@ronaksingh27
Copy link
Contributor

@jellyfishcake Could you please assign me this issue? I'd also appreciate any insights or suggestions on where I should start

@jellyfishcake
Copy link
Contributor Author

@jellyfishcake Could you please assign me this issue? I'd also appreciate any insights or suggestions on where I should start

Assigned. I just start with the /linux_benchmark files or /linux_packages files.

@ronaksingh27
Copy link
Contributor

ronaksingh27 commented Oct 8, 2024

@jellyfishcake I am currently working on adding type annotations for the aerospike_benchmarks.py file in the Linux benchmarks. However, I am having difficulty with the type annotation for user_config. Could you please assist me with this?

@ronaksingh27
Copy link
Contributor

@jellyfishcake How do i build the intuition that this parameter would have this type annotation ?

@ronaksingh27
Copy link
Contributor

@jellyfishcake How do i build the intuition that this parameter would have this type annotation ?
@jellyfishcake
I did find that certain files already have the function with the type annotations like

cuda_memcpy_benchmark.py already had , so i applied them to aerospike_benchmark.py , is that all right ?

@ronaksingh27
Copy link
Contributor

ronaksingh27 commented Oct 8, 2024

@jellyfishcake I am currently working on adding type annotations for the aerospike_benchmarks.py file in the Linux benchmarks. However, I am having difficulty with the type annotation for user_config. Could you please assist me with this?

well i got this from the same cuda_memcpy_benchamark.py file :)

@ronaksingh27
Copy link
Contributor

ronaksingh27 commented Oct 8, 2024

@jellyfishcake I had the file aerospike_benchmark.py updated and raised the Pull Request , could please provide feedback, Thanks for the response earlier for suggesting where to start from , it helped a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This is a good issue for someone new to PKB
Projects
None yet
Development

No branches or pull requests

2 participants