Skip to content

Commit

Permalink
refine docstring (#344)
Browse files Browse the repository at this point in the history
# Description

Please add an informative description that covers that changes made by
the pull request and link all relevant issues.

# All Promptflow Contribution checklist:
- [ ] **The pull request does not introduce [breaking changes]**
- [ ] **CHANGELOG is updated for new features, bug fixes or other
significant changes.**
- [ ] **I have read the [contribution guidelines](../CONTRIBUTING.md).**

## General Guidelines and Best Practices
- [ ] Title of the pull request is clear and informative.
- [ ] There are a small number of commits, each of which have an
informative message. This means that previously merged commits do not
appear in the history of the PR. For more information on cleaning up the
commits in your PR, [see this
page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).

### Testing Guidelines
- [ ] Pull request includes test coverage for the included changes.
  • Loading branch information
wangchao1230 authored Sep 8, 2023
1 parent 58def61 commit b02229a
Show file tree
Hide file tree
Showing 16 changed files with 67 additions and 670 deletions.
14 changes: 9 additions & 5 deletions src/promptflow/promptflow/_sdk/entities/_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import json
from os import PathLike
from pathlib import Path
from typing import Dict, Union
from typing import Dict, Union, List

from promptflow._sdk._constants import (
BASE_PATH_CONTEXT_KEY,
Expand Down Expand Up @@ -102,7 +102,8 @@ def _casting_type(cls, typ):
return type_dict.get(typ)
return snake_to_camel(typ)

def keys(self):
def keys(self) -> List:
"""Return keys of the connection properties."""
return list(self.configs.keys()) + list(self.secrets.keys())

def __getitem__(self, item):
Expand Down Expand Up @@ -236,7 +237,7 @@ def _load(
)
return connection

def to_execution_connection_dict(self) -> dict:
def _to_execution_connection_dict(self) -> dict:
value = {**self.configs, **self.secrets}
secret_keys = list(self.secrets.keys())
return {
Expand All @@ -247,7 +248,7 @@ def to_execution_connection_dict(self) -> dict:
}

@classmethod
def from_execution_connection_dict(cls, name, data) -> "_Connection":
def _from_execution_connection_dict(cls, name, data) -> "_Connection":
type_cls, _ = cls._resolve_cls_and_type(data={"type": data.get("type")[: -len("Connection")]})
value_dict = data.get("value", {})
if type_cls == CustomConnection:
Expand Down Expand Up @@ -289,10 +290,12 @@ def _from_orm_object_with_secrets(cls, orm_object: ORMConnection):

@property
def api_key(self):
return self.secrets.get("api_key", "***")
"""Return the api key."""
return self.secrets.get("api_key", SCRUBBED_VALUE)

@api_key.setter
def api_key(self, value):
"""Set the api key."""
self.secrets["api_key"] = value


Expand Down Expand Up @@ -644,6 +647,7 @@ def __getattr__(self, item):
return super().__getattribute__(item)

def is_secret(self, item):
"""Check if item is a secret."""
# Note: This is added for compatibility with promptflow.connections custom connection usage.
return item in self.secrets

Expand Down
2 changes: 1 addition & 1 deletion src/promptflow/promptflow/_sdk/entities/_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def _get_local_connections(cls, executable):
for n in connection_names:
try:
conn = local_client.connections.get(name=n, with_secrets=True)
result[n] = conn.to_execution_connection_dict()
result[n] = conn._to_execution_connection_dict()
except ConnectionNotFoundError:
# ignore when connection not found since it can be configured with env var.
raise Exception(f"Connection {n!r} required for flow {executable.name!r} is not found.")
Expand Down
4 changes: 2 additions & 2 deletions src/promptflow/promptflow/_sdk/operations/_flow_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test(
node: str = None,
environment_variables: dict = None,
) -> dict:
"""Test flow or node
"""Test flow or node.
:param flow: path to flow directory to test
:type flow: Union[str, PathLike]
Expand Down Expand Up @@ -84,7 +84,7 @@ def _test(
stream_log: bool = True,
allow_generator_output: bool = True,
):
"""Test flow or node
"""Test flow or node.
:param flow: path to flow directory to test
:param inputs: Input data for the flow test
Expand Down
27 changes: 19 additions & 8 deletions src/promptflow/promptflow/_sdk/operations/_run_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ def list(
message_generator=lambda x: f"Error parsing run {x.name!r}, skipped.",
)

@classmethod
def get(cls, name: str) -> Run:
def get(self, name: str) -> Run:
"""Get a run entity.
:param name: Name of the run.
Expand Down Expand Up @@ -180,6 +179,13 @@ def update(
return self.get(name)

def get_details(self, name: Union[str, Run]) -> pd.DataFrame:
"""Get run inputs and outputs.
:param name: name of the run.
:type name: str
:return: Run details.
:rtype: ~pandas.DataFrame
"""
name = Run._validate_and_return_run_name(name)
run = self.get(name=name)
run._check_run_status_is_completed()
Expand All @@ -200,6 +206,13 @@ def get_details(self, name: Union[str, Run]) -> pd.DataFrame:
return df

def get_metrics(self, name: Union[str, Run]) -> Dict[str, Any]:
"""Get run metrics.
:param name: name of the run.
:type name: str
:return: Run metrics.
:rtype: Dict[str, Any]
"""
name = Run._validate_and_return_run_name(name)
run = self.get(name=name)
run._check_run_status_is_completed()
Expand Down Expand Up @@ -253,18 +266,16 @@ def visualize(self, runs: Union[str, Run, List[str], List[Run]], **kwargs) -> No
error_message = f"Cannot visualize non-completed run. {str(e)}"
logger.error(error_message)

@classmethod
def _get_outputs(cls, run: Union[str, Run]) -> List[Dict[str, Any]]:
def _get_outputs(self, run: Union[str, Run]) -> List[Dict[str, Any]]:
"""Get the outputs of the run, load from local storage."""
if isinstance(run, str):
run = cls.get(name=run)
run = self.get(name=run)
local_storage = LocalStorageOperations(run)
return local_storage.load_outputs()

@classmethod
def _get_inputs(cls, run: Union[str, Run]) -> List[Dict[str, Any]]:
def _get_inputs(self, run: Union[str, Run]) -> List[Dict[str, Any]]:
"""Get the outputs of the run, load from local storage."""
if isinstance(run, str):
run = cls.get(name=run)
run = self.get(name=run)
local_storage = LocalStorageOperations(run)
return local_storage.load_inputs()
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def resolve_connection_names(connection_names, raise_error=False):
for n in connection_names:
try:
conn = local_client.connections.get(name=n, with_secrets=True)
result[n] = conn.to_execution_connection_dict()
result[n] = conn._to_execution_connection_dict()
except Exception as e:
if raise_error:
raise e
Expand Down
59 changes: 0 additions & 59 deletions src/promptflow/promptflow/azure/_configuration.py

This file was deleted.

3 changes: 1 addition & 2 deletions src/promptflow/promptflow/azure/operations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
__path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore


from ._flow_opearations import FlowOperations
from ._run_operations import RunOperations

__all__ = ["FlowOperations", "RunOperations"]
__all__ = ["RunOperations"]
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@


class ConnectionOperations(_ScopeDependentOperations):
"""FlowOperations.
"""ConnectionOperations.
You should not instantiate this class directly. Instead, you should
create an MLClient instance that instantiates it for you and
create an PFClient instance that instantiates it for you and
attaches it as an attribute.
"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@


class FlowOperations(_ScopeDependentOperations):
"""FlowOperations.
"""FlowOperations that can manage flows.
You should not instantiate this class directly. Instead, you should
create an MLClient instance that instantiates it for you and
attaches it as an attribute
create an PFClient instance that instantiates it for you and
attaches it as an attribute.
"""

def __init__(
Expand Down
29 changes: 25 additions & 4 deletions src/promptflow/promptflow/azure/operations/_run_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def __init__(self, message):


class RunOperations(_ScopeDependentOperations):
"""FlowRunOperations.
"""RunOperations that can manage runs.
You should not instantiate this class directly. Instead, you should
create an PFClient instance that instantiates it for you and
Expand Down Expand Up @@ -175,6 +175,13 @@ def _get_headers(self):
return custom_header

def create_or_update(self, run: Run, **kwargs) -> Run:
"""Create or update a run.
:param run: Run object to create or update.
:type run: ~promptflow.entities.Run
:return: Run object created or updated.
:rtype: ~promptflow.entities.Run
"""
stream = kwargs.pop("stream", False)
reset = kwargs.pop("reset_runtime", False)

Expand Down Expand Up @@ -423,10 +430,24 @@ def _get_run_from_index_service(self, flow_run_id, **kwargs):
f"Failed to get run metrics from service. Code: {response.status_code}, text: {response.text}"
)

def archive(self, run_name):
def archive(self, run: str) -> Run:
"""Archive a run.
:param run: The run name
:type run: str
:return: The run
:rtype: Run
"""
pass

def restore(self, run_name):
def restore(self, run: str) -> Run:
"""Restore a run.
:param run: The run name
:type run: str
:return: The run
:rtype: Run
"""
pass

def _get_log(self, flow_run_id: str) -> str:
Expand Down Expand Up @@ -586,7 +607,7 @@ def _get_inputs_outputs_from_child_runs(self, runs: List[Dict[str, Any]]):
return inputs, outputs

def visualize(self, runs: Union[str, Run, List[str], List[Run]], **kwargs) -> None:
"""Visualize run(s).
"""Visualize run(s) using Azure AI portal.
:param runs: Names of the runs, or list of run objects.
:type runs: Union[str, ~promptflow.sdk.entities.Run, List[str], List[~promptflow.sdk.entities.Run]]
Expand Down
2 changes: 1 addition & 1 deletion src/promptflow/tests/sdk_cli_test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def setup_local_connection(local_client):
for name, _dct in connection_dict.items():
if _dct["type"] == "BingConnection":
continue
local_client.connections.create_or_update(_Connection.from_execution_connection_dict(name=name, data=_dct))
local_client.connections.create_or_update(_Connection._from_execution_connection_dict(name=name, data=_dct))
_connection_setup = True


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def test_connection_load_from_env_file_bad_case(self):
def test_to_execution_connection_dict(self):
# Assert custom connection build
connection = CustomConnection(name="test_connection", configs={"a": "1"}, secrets={"b": "2"})
assert connection.to_execution_connection_dict() == {
assert connection._to_execution_connection_dict() == {
"module": "promptflow.connections",
"secret_keys": ["b"],
"type": "CustomConnection",
Expand All @@ -213,7 +213,7 @@ def test_to_execution_connection_dict(self):
api_type="azure",
api_version="2023-07-01-preview",
)
assert connection.to_execution_connection_dict() == {
assert connection._to_execution_connection_dict() == {
"module": "promptflow.connections",
"secret_keys": ["api_key"],
"type": "AzureOpenAIConnection",
Expand All @@ -232,7 +232,7 @@ def test_to_execution_connection_dict(self):
api_key="test_key",
organization="test_org",
)
assert connection.to_execution_connection_dict() == {
assert connection._to_execution_connection_dict() == {
"module": "promptflow.connections",
"secret_keys": ["api_key"],
"type": "OpenAIConnection",
Expand Down
Loading

0 comments on commit b02229a

Please sign in to comment.