Skip to content

Create Controller.walk_attributes(access_mode=) helper#138

Closed
shihab-dls wants to merge 3 commits intomainfrom
112_walk_attributes_helper
Closed

Create Controller.walk_attributes(access_mode=) helper#138
shihab-dls wants to merge 3 commits intomainfrom
112_walk_attributes_helper

Conversation

@shihab-dls
Copy link
Contributor

Fixes #112

This PR adds a Controller method to return all attributes of a given access mode, wherein AttrW and AttrR are subsets of AttrRW. A method was also added to return methods of a given access mode (excluding dunders).

@codecov
Copy link

codecov bot commented Mar 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.74%. Comparing base (5aa7a12) to head (31e9781).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #138      +/-   ##
==========================================
+ Coverage   90.58%   90.74%   +0.15%     
==========================================
  Files          41       41              
  Lines        1933     1955      +22     
==========================================
+ Hits         1751     1774      +23     
+ Misses        182      181       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shihab-dls shihab-dls requested a review from GDYendell March 17, 2025 12:01
Copy link
Contributor

@evvaaaa evvaaaa Mar 17, 2025

Choose a reason for hiding this comment

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

Can we use a TypeVar bound to attributes here?

Suggested change
def walk_attributes(self, access_mode: type[Attribute]):
def walk_attributes(self, access_mode: type[T_Attribute] = Attribute) -> dict[str, T_Attribute]:

Copy link
Contributor

Choose a reason for hiding this comment

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

What python version do we need for this syntax?

Copy link
Contributor

@evvaaaa evvaaaa Mar 17, 2025

Choose a reason for hiding this comment

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

Tested on 3.11:

class X():
    x: int

class X1(X):
    x1: int
    
class X2(X):
    x2: int

class X3(X2):
    x3: int

T = TypeVar('T', bound=X)

def foo(xs: dict[str, T], x_t: type[T] = X) -> dict[str, T]:
    return {k: v for k, v in xs.items() if isinstance(v, x_t)}

xs = {"a": X1(), "b": X2(), "c": X3()}

for k, v in foo(xs, x_t=X3).items():
    # These work
    print(k, v.x)
    print(k, v.x2)
    print(k, v.x3)

    # attribute "x1" is unknown pyright error
    print(k, v.x1)

Comment on lines 131 to 137
Copy link
Contributor

@evvaaaa evvaaaa Mar 17, 2025

Choose a reason for hiding this comment

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

I think we're looking for cs_methods on the controller, e.g

class Command(Method[BaseController]):

class Scan(Method[BaseController]):

class Put(Method[BaseController]):

with

def walk_methods(self, access_mode: type[Method] = Method) -> dict[str, Method]

Comment on lines 123 to 128
Copy link
Contributor

Choose a reason for hiding this comment

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

Be nice for this to be an optional restriction:

Suggested change
def walk_attributes(self, access_mode: type[Attribute]):
return {
attr: values
for attr, values in self.attributes.items()
if isinstance(values, access_mode)
}
def walk_attributes(self, access_mode: type[Attribute] = Attribute):
return {
name: attribute
for name, attribute in self.attributes.items()
if isinstance(attribute, access_mode)
}

@shihab-dls
Copy link
Contributor Author

@evalott100 mentioned protocols, so I've added in a protocol for Method in controller.py, to avoid the circular dependency, but maybe this should live somewhere else (or possibly I've misunderstood).

@evvaaaa
Copy link
Contributor

evvaaaa commented Mar 18, 2025

Closing as we no longer need this functionality.

@evvaaaa evvaaaa closed this Mar 18, 2025
@GDYendell GDYendell deleted the 112_walk_attributes_helper branch December 9, 2025 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create Controller.walk_attributes(access_mode=) helper

3 participants

Comments