Skip to content

MNT: Final Annotation #63

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

Merged
merged 2 commits into from
Apr 22, 2025

Conversation

welli7ngton
Copy link
Contributor

  • add annotation to the Application session and '_fix_retina_element' function.

@welli7ngton
Copy link
Contributor Author

I wonder what would be the right annotation to Backend.WIN_32, and the dict annotation would be enough for the **selectors?

I've used this docs as reference.

@@ -230,7 +230,7 @@ def _to_dict(lbs, elems):
else:
return _to_dict(labels, results)

def _fix_retina_element(self, ele):
def _fix_retina_element(self, ele: cv2find.Box) -> cv2find.Box:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this Box type was a good option.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's okay to use it this way, but I can do some tests to validate it.

@joao-voltarelli
Copy link
Contributor

Hi @welli7ngton!

Just a few notes about the type annotations in the "Application" section.

I think that in the annotation for the backend parameter, we can use the Backend class itself. This class is just a string enum with the options that can be used; in this case it will be Backend.WIN_32 (by default) or Backend.UIA.

backend: Backend = Backend.WIN_32

Here are more details:


Regarding the appropriate type for **selectors, since they represent arbitrary named arguments (**kwargs), I think using just 'dict' would not be ideal since, in this case, it would indicate that **selectors are a single dictionary.

Since these parameters refer to element selectors (reference here), they can be of different types (str, int, bool, etc.). So, in this case, an option could use the type notation like: dict[str, Any]. This indicates that each passed selector will be considered a dictionary where the key is a string (selector name), and the value can be of any type.

**connection_selectors: dict[str, Any]

Newer versions of Python seem to have some more efficient ways of doing this type annotation for **kwargs, but I think they are not compatible with older versions of Python. Here are some references I found:

So at least for now, I think there would be no problem using it in the way I mentioned.

@joao-voltarelli
Copy link
Contributor

@welli7ngton Merging this PR with the last type annotations. Thanks for the contribution!

@joao-voltarelli joao-voltarelli merged commit 7f72a69 into botcity-dev:main Apr 22, 2025
3 checks passed
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.

2 participants