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

chore: apply flake8 builtin rules #248

Merged
merged 10 commits into from
Jul 30, 2024
2 changes: 1 addition & 1 deletion examples/connect/shiny-python/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
app_ui = ui.page_fluid(ui.output_text("text"), ui.output_data_frame("result"))


def server(input: Inputs, output: Outputs, session: Session):
def server(i: Inputs, o: Outputs, session: Session):
"""
Shiny for Python example application that shows user information and
the first few rows from a table hosted in Databricks.
Expand Down
29 changes: 27 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,32 @@ docstring-code-format = true
docstring-code-line-length = "dynamic"

[tool.ruff.lint]
select = ["D", "F401", "I"]
select = [
# flake8-builtins
# https://docs.astral.sh/ruff/rules/#flake8-builtins-a
#
# Check for builtin shadowing (i.e., naming a variable 'for', which is a builtin.)
"A",

# pydocstyle
# https://docs.astral.sh/ruff/rules/#pydocstyle-d
# https://docs.astral.sh/ruff/faq/#does-ruff-support-numpy-or-google-style-docstrings
#
# Check docstring formatting. Many of these rules are intentionally ignored below.
"D",

# pyflakes - unused-import
# https://docs.astral.sh/ruff/rules/unused-import/
#
# Check for unused imports.
"F401",

# isort
# https://docs.astral.sh/ruff/rules/#isort-i
#
# Sort imports.
"I"
]
ignore = [
# NumPy style docstring convention with noted exceptions.
# https://docs.astral.sh/ruff/faq/#does-ruff-support-numpy-or-google-style-docstrings
Expand All @@ -67,7 +92,7 @@ ignore = [
'D415',
'D416',
'D417',
'D418', # The Python Language Server can accomdate documentation for individual methods.
'D418', # The Python Language Server can accomodate documentation for individual methods.
# TODO(#135) resarch D418 and determine if we should continue ignoring it.
]

Expand Down
22 changes: 11 additions & 11 deletions src/posit/connect/bundles.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,16 +235,16 @@ def __init__(
super().__init__(params)
self.content_guid = content_guid

def create(self, input: io.BufferedReader | bytes | str) -> Bundle:
def create(self, archive: io.BufferedReader | bytes | str) -> Bundle:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this to avoid using input as an argument?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct. Since input is a builtin function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We might not need to worry cause this is internal enough or no one is yet using it, but does this change have user-facing implications?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it does if anyone is using kwargs when calling this method. Since it's early and we are pre-1.0.0, I am ok releasing this under 0.4.0 along with documentation of the breaking changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And for my own edification: documentation of those breaking changes isn't in this PR yeah? That'll be a follow on?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think release notes are the correct place to write them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I started a migration guide in the changelog: https://github.com/posit-dev/posit-sdk-py/releases

"""
Create a bundle.

Create a bundle from a file or memory.

Parameters
----------
input : io.BufferedReader, bytes, or str
Input archive for bundle creation. A 'str' type assumes a relative or absolute filepath.
archive : io.BufferedReader, bytes, or str
Archive for bundle creation. A 'str' type assumes a relative or absolute filepath.

Returns
-------
Expand Down Expand Up @@ -273,14 +273,14 @@ def create(self, input: io.BufferedReader | bytes | str) -> Bundle:
>>> bundle.create("bundle.tar.gz")
None
"""
if isinstance(input, (io.BufferedReader, bytes)):
data = input
elif isinstance(input, str):
with open(input, "rb") as file:
if isinstance(archive, (io.BufferedReader, bytes)):
data = archive
elif isinstance(archive, str):
with open(archive, "rb") as file:
data = file.read()
else:
raise TypeError(
f"create() expected argument type 'io.BufferedReader', 'bytes', or 'str', but got '{type(input).__name__}'"
f"create() expected argument type 'io.BufferedReader', 'bytes', or 'str', but got '{type(archive).__name__}'"
)

path = f"v1/content/{self.content_guid}/bundles"
Expand Down Expand Up @@ -314,20 +314,20 @@ def find_one(self) -> Bundle | None:
bundles = self.find()
return next(iter(bundles), None)

def get(self, id: str) -> Bundle:
def get(self, uid: str) -> Bundle:
"""Get a bundle.

Parameters
----------
id : str
uid : str
Identifier of the bundle to retrieve.

Returns
-------
Bundle
The bundle with the specified ID.
"""
path = f"v1/content/{self.content_guid}/bundles/{id}"
path = f"v1/content/{self.content_guid}/bundles/{uid}"
url = self.url + path
response = self.session.get(url)
result = response.json()
Expand Down
12 changes: 6 additions & 6 deletions src/posit/connect/cursors.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,17 @@ def fetch_pages(self) -> Generator[CursorPage, None, None]:
------
Generator[Page, None, None]
"""
next = None
next_page = None
while True:
page = self.fetch_page(next)
page = self.fetch_page(next_page)
yield page
cursors: dict = page.paging.get("cursors", {})
next = cursors.get("next")
if not next:
next_page = cursors.get("next")
if not next_page:
# stop if a next page is not defined
return

def fetch_page(self, next: str | None = None) -> CursorPage:
def fetch_page(self, next_page: str | None = None) -> CursorPage:
"""Fetch a page.

Parameters
Expand All @@ -69,7 +69,7 @@ def fetch_page(self, next: str | None = None) -> CursorPage:
"""
params = {
**self.params,
"next": next,
"next": next_page,
"limit": _MAX_PAGE_SIZE,
}
response = self.session.get(self.url, params=params)
Expand Down
6 changes: 3 additions & 3 deletions src/posit/connect/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,19 +149,19 @@ def find_one(self, *args, **kwargs) -> Permission | None:
permissions = self.find(*args, **kwargs)
return next(iter(permissions), None)

def get(self, id: str) -> Permission:
def get(self, uid: str) -> Permission:
"""Get a permission.

Parameters
----------
id : str
uid : str
The permission id.

Returns
-------
Permission
"""
path = f"v1/content/{self.content_guid}/permissions/{id}"
path = f"v1/content/{self.content_guid}/permissions/{uid}"
url = self.url + path
response = self.session.get(url)
return Permission(self.params, **response.json())
14 changes: 7 additions & 7 deletions src/posit/connect/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,12 @@ def wait_for(self) -> None:

class Tasks(resources.Resources):
@overload
def get(self, id: str, first: int, wait: int) -> Task:
def get(self, uid: str, first: int, wait: int) -> Task:
"""Get a task.

Parameters
----------
id : str
uid : str
Task identifier.
first : int, default 0
Line to start output on.
Expand All @@ -163,12 +163,12 @@ def get(self, id: str, first: int, wait: int) -> Task:
...

@overload
def get(self, id: str, *args, **kwargs) -> Task:
def get(self, uid: str, *args, **kwargs) -> Task:
"""Get a task.

Parameters
----------
id : str
uid : str
Task identifier.

Returns
Expand All @@ -177,20 +177,20 @@ def get(self, id: str, *args, **kwargs) -> Task:
"""
...

def get(self, id: str, *args, **kwargs) -> Task:
def get(self, uid: str, *args, **kwargs) -> Task:
"""Get a task.

Parameters
----------
id : str
uid : str
Task identifier.

Returns
-------
Task
"""
params = dict(*args, **kwargs)
path = f"v1/tasks/{id}"
path = f"v1/tasks/{uid}"
url = self.url + path
response = self.session.get(url, params=params)
result = response.json()
Expand Down
4 changes: 2 additions & 2 deletions src/posit/connect/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ def find_one(self, *args, **kwargs) -> User | None:
)
return next(users, None)

def get(self, id: str) -> User:
url = self.url + f"v1/users/{id}"
def get(self, uid: str) -> User:
url = self.url + f"v1/users/{uid}"
response = self.session.get(url)
return User(
self.params,
Expand Down
6 changes: 3 additions & 3 deletions tests/posit/connect/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@
from .api import load_mock # type: ignore


@pytest.fixture
@pytest.fixture()
def MockAuth():
with patch("posit.connect.client.Auth") as mock:
yield mock


@pytest.fixture
@pytest.fixture()
def MockConfig():
with patch("posit.connect.client.Config") as mock:
yield mock


@pytest.fixture
@pytest.fixture()
def MockSession():
with patch("posit.connect.client.Session") as mock:
yield mock
Expand Down
4 changes: 2 additions & 2 deletions tests/posit/connect/test_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,10 @@ def test_find():
content = c.content.get(guid)

# invoke
vars = content.environment_variables.find()
envvars = content.environment_variables.find()

# assert
assert vars == ["TEST"]
assert envvars == ["TEST"]
assert mock_get_content.call_count == 1
assert mock_get_environment.call_count == 1

Expand Down
34 changes: 17 additions & 17 deletions tests/posit/connect/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ class TestPermissionDelete:
@responses.activate
def test(self):
# data
id = "94"
uid = "94"
content_guid = "f2f37341-e21d-3d80-c698-a935ad614066"

# behavior
mock_delete = responses.delete(
f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{id}"
f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{uid}"
)

# setup
params = ResourceParameters(
requests.Session(), Url("https://connect.example/__api__")
)
fake_permission = load_mock(
f"v1/content/{content_guid}/permissions/{id}.json"
f"v1/content/{content_guid}/permissions/{uid}.json"
)
permission = Permission(params, **fake_permission)

Expand All @@ -43,7 +43,7 @@ class TestPermissionUpdate:
@responses.activate
def test_request_shape(self):
# test data
id = random.randint(0, 100)
uid = random.randint(0, 100)
content_guid = str(uuid.uuid4())
principal_guid = str(uuid.uuid4())
principal_type = "principal_type"
Expand All @@ -52,7 +52,7 @@ def test_request_shape(self):

# define api behavior
responses.put(
f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{id}",
f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{uid}",
json={
# doesn't matter for this test
},
Expand All @@ -77,7 +77,7 @@ def test_request_shape(self):
)
permission = Permission(
params,
id=id,
id=uid,
content_guid=content_guid,
principal_guid=principal_guid,
principal_type=principal_type,
Expand All @@ -94,18 +94,18 @@ def test_role_update(self):
old_role = "owner"
new_role = "viewer"

id = "94"
uid = "94"
content_guid = "f2f37341-e21d-3d80-c698-a935ad614066"
fake_permission = load_mock(
f"v1/content/{content_guid}/permissions/{id}.json"
f"v1/content/{content_guid}/permissions/{uid}.json"
)
fake_permission.update(role=new_role)

# define api behavior
id = random.randint(0, 100)
uid = random.randint(0, 100)
content_guid = str(uuid.uuid4())
responses.put(
f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{id}",
f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{uid}",
json=fake_permission,
match=[
matchers.json_params_matcher(
Expand All @@ -123,7 +123,7 @@ def test_role_update(self):
requests.Session(), Url("https://connect.example/__api__")
)
permission = Permission(
params, id=id, content_guid=content_guid, role=old_role
params, id=uid, content_guid=content_guid, role=old_role
)

# assert role change with respect to api response
Expand Down Expand Up @@ -164,13 +164,13 @@ class TestPermissionsCreate:
@responses.activate
def test(self):
# data
id = "94"
uid = "94"
content_guid = "f2f37341-e21d-3d80-c698-a935ad614066"
principal_guid = str(uuid.uuid4())
principal_type = "user"
role = "owner"
fake_permission = {
**load_mock(f"v1/content/{content_guid}/permissions/{id}.json"),
**load_mock(f"v1/content/{content_guid}/permissions/{uid}.json"),
"principal_guid": principal_guid,
"principal_type": principal_type,
"role": role,
Expand Down Expand Up @@ -268,15 +268,15 @@ class TestPermissionsGet:
@responses.activate
def test(self):
# data
id = "94"
uid = "94"
content_guid = "f2f37341-e21d-3d80-c698-a935ad614066"
fake_permission = load_mock(
f"v1/content/{content_guid}/permissions/{id}.json"
f"v1/content/{content_guid}/permissions/{uid}.json"
)

# behavior
responses.get(
f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{id}",
f"https://connect.example/__api__/v1/content/{content_guid}/permissions/{uid}",
json=fake_permission,
)

Expand All @@ -287,7 +287,7 @@ def test(self):
permissions = Permissions(params, content_guid=content_guid)

# invoke
permission = permissions.get(id)
permission = permissions.get(uid)

# assert
assert permission == fake_permission
Loading
Loading