Skip to content

MNT: Add type hints to Mouse methods #61

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

Conversation

welli7ngton
Copy link
Contributor

  • Added explicit return types to various DesktopBot methods.
  • Introduced Literal for button parameters to restrict values:
    • I realized that the Button type is an Enum, from now on the bot only accepts the literal values of left, right and middle because I didn't find a better way to represent this enum, what do you guys think.
  • Ensured consistent type annotations across mouse-related functions.

- Added explicit return types to various DesktopBot methods.
- Introduced Literal for button parameters to restrict values:
   - I realized that the Button type is an Enum, from now on the bot only accepts the literal values of left, right and middle because I didn't find a better way to represent this enum, what do you guys think.
- Ensured consistent type annotations across mouse-related functions.
@joao-voltarelli
Copy link
Contributor

Hi @welli7ngton, great work!

I think using Literal could work well for this scenario. There's just one small problem, if I'm not mistaken, the 'Literal' type is available from Python >= 3.8. Since the framework supports Python 3.7, some older projects that still use Python 3.7 may be incompatible with future versions.

I was taking a better look at the _mouse_click method, and it has an internal check to verify the value that was passed to the 'button' parameter, so if the user passes an incorrect value, the method itself will throw an exception indicating the problem.

So, I believe that leaving the parameter type as 'str' for now won't be a big problem since the method checks the content that was passed. What do you think?

Here's a reference: https://github.com/botcity-dev/botcity-framework-core-python/blob/815196110f09b028b9546c0af8bc523cf1ecc8d8/botcity/core/input_utils.py#L77#L80

@welli7ngton
Copy link
Contributor Author

Hello @joao-voltarelli

You're right, Literal is only avaible from Python 3.8 onwards, i found a way to use Literal type in older versions of python, i havents tried yet, but heres the example that i found on stackoverflow: ImportError: cannot import name 'Literal' from 'typing'

Let me know what do you think

@joao-voltarelli
Copy link
Contributor

Hey @welli7ngton!

Yesterday, I was reading about the typing_extensions package, and it seemed like an interesting alternative, but I got a bit confused when reading the oficial documentation.

Apparently, although it seems to work for older versions of Python, the official documentation mentions that it currently supports version 3.8 or higher and will stop supporting older versions in the future: https://typing-extensions.readthedocs.io/en/latest/#python-version-support

So I'm not really sure how it works. If you can, could you please look into it and take a deeper look into how it works?

@welli7ngton
Copy link
Contributor Author

@joao-voltarelli Sure, I'll dive into it. If I don't get confident that everything will work smoothly without issues, I'll proceed with your last proposed solution.

@welli7ngton
Copy link
Contributor Author

Hi @joao-voltarelli, that doc sounds kinda confuse for me, im gonna proceed with your solution :))

- Added explicit return types to various DesktopBot methods.
- Introduced Literal for button parameters to restrict values:
   - I realized that the Button type is an Enum, from now on the bot only accepts the literal values of left, right and middle because I didn't find a better way to represent this enum, what do you guys think.
- Ensured consistent type annotations across mouse-related functions.
- Removed 'Literal' type hints for mouse button parameters in favor of str for flexibility.
- Ensured mouse button values are maaped correctly using mouse_map.get() with a defalt fallback 'left'.
- Reformatted dict definitions and functions parameters for readbility and consistency.
- Standardized string quoting style and fixed minor inconsistencies (double white spaces, single quotes mixed with double quotes).
@welli7ngton
Copy link
Contributor Author

Hey @joao-voltarelli, i had to solve a conflict in my branches.

I didn't understand the reason for the conflict when trying to push my branch, apparently it was resolved, but if you find it interesting that I redo the branch to avoid any problems, I understand and make the changes from the most recent version of the main branch.

Best regards, welli7ngton.

@joao-voltarelli
Copy link
Contributor

Hi @joao-voltarelli, that doc sounds kinda confuse for me, im gonna proceed with your solution :))

No problem following this way! Since the method already has an internal check of the content that is passed, there is no problem if we leave the type as "str" ​​for now.

@@ -878,22 +878,25 @@ def click(
button (str, optional): One of 'left', 'right', 'middle'. Defaults to 'left'
"""
x, y = self.state.center()

mouse_button = mouse_map.get(button, "left")
Copy link
Contributor

Choose a reason for hiding this comment

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

@welli7ngton I think we'll just need to make a small adjustment.

In the click(), click_relative(), mouse_down() and mouse_up() methods we can remove this treatment:

  • mouse_button = mouse_map.get(button, "left")

I say this because the _mouse_click() method that is invoked expects the parameter to be of type 'str'. This treatment means the mouse_button variable can be an Enum (if it exists in the mouse_map dictionary).

With this, the internal check that is done within the _mouse_click() method would cause the ValueError exception to be thrown: https://github.com/botcity-dev/botcity-framework-core-python/blob/815196110f09b028b9546c0af8bc523cf1ecc8d8/botcity/core/input_utils.py#L77#L80

Since this parameter value check is already done in _mouse_click(), I think we can leave it out of the methods in bot.py. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @joao-voltarelli, i see now, it also ensure consistency in handling mouse button inputs. I agree, ill solve this soon, thanks for the feedback

welli7ngton and others added 3 commits February 15, 2025 12:58
duplicate treatment from the functions click(), click_relative(), mouse_down() and mouse_up()
@joao-voltarelli
Copy link
Contributor

Hi @welli7ngton, now everything likes to work fine!

Just one detail, I gave you incorrect information about the mouse_down() and mouse_up() methods.
In these two specific methods, we need to use this treatment to get the correct button:
mouse_button = mouse_map.get(button, None), since the pyinput press and release methods do not accept strings.

Sorry about that! I've already made a commit to fix this point, thanks for the contribution!

@joao-voltarelli joao-voltarelli merged commit e9ddca5 into botcity-dev:main Feb 17, 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