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.

Sign up for GitHub

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
Merged

chore: apply flake8 builtin rules #248

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
019ba4c
refactor: class facade for urls module
tdstein Jul 26, 2024
48a1708
refactor: mark urls module methods as internal
tdstein Jul 26, 2024
c17dcfa
docs: add docstrings for url class
tdstein Jul 26, 2024
ba04863
refactor: optimizations
tdstein Jul 26, 2024
c2a720f
build: add import formatting with autoflake and isort
tdstein Jul 26, 2024
a98f756
use ruff instead of isort, and combine make fmt and make fix.
tdstein Jul 26, 2024
844379f
move ruff rule selection to pyproject.toml
tdstein Jul 26, 2024
00396a3
replace autoflake8 with ruff rule F401
tdstein Jul 26, 2024
571177b
chore: apply flake8 builtin rules
tdstein Jul 26, 2024
00444ad
add comments to linting rules
tdstein Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/connect/shiny-python/app.py
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ docstring-code-format = true
docstring-code-line-length = "dynamic"

[tool.ruff.lint]
select = ["D", "F401", "I"]
select = ["A", "D", "F401", "I"]
tdstein marked this conversation as resolved.
Show resolved Hide resolved
ignore = [
# NumPy style docstring convention with noted exceptions.
# https://docs.astral.sh/ruff/faq/#does-ruff-support-numpy-or-google-style-docstrings
Expand Down
22 changes: 11 additions & 11 deletions src/posit/connect/bundles.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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