-
Notifications
You must be signed in to change notification settings - Fork 112
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
Implement type stubs for ecodes #216
base: main
Are you sure you want to change the base?
Conversation
Helps your $EDITOR find the constants from ecodes.c, making for a more pleasant developer experience.
Hey, sorry I haven't spent time on this yet. I hope I'll manage to look into this soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, thanks for waiting.
Hm, makes me wonder why setup.py is generating ecodes.c, and not ecodes.py.
Anyhow, your change works for me 👍.
A few questions/discussions, and a suggestion for a refactoring of genpyi.py.
Turns out that pycharm also generates stubs by the way, from /usr/lib/python3/dist-packages/evdev/_ecodes.cpython-312-x86_64-linux-gnu.so
, putting them into ~/.cache/JetBrains/PyCharmCE2023.2/python_stubs/-32419094/evdev/_ecodes.py
.
# I've been reading through setuptools docs, but i can't for the lofe of me figure out how to bring the pyi stubs into the package "cleanly" | ||
class install(_install.install): | ||
def run(self): | ||
print(os.listdir(os.getenv("src"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does the "src" environment variable come from? Is it something from when you were developing the change for debugging or something?
def run(self): | ||
print(os.listdir(os.getenv("src"))) | ||
#_install.install.copy_file(self, "evdev/ecodes.pyi", "evdev/ecodes.pyi") | ||
installdir = "build/lib.linux-x86_64-cpython-311/evdev" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
installdir
should probably be os.path.join(self.build_lib, 'evdev')
#! /usr/bin/env python3 | ||
""" | ||
Generate a Python extension module with the constants defined in linux/input.h. | ||
""" | ||
|
||
from __future__ import print_function | ||
import os, sys, re | ||
|
||
|
||
# ----------------------------------------------------------------------------- | ||
# The default header file locations to try. | ||
headers = [ | ||
"/usr/include/linux/input.h", | ||
"/usr/include/linux/input-event-codes.h", | ||
"/usr/include/linux/uinput.h", | ||
] | ||
|
||
if sys.argv[1:]: | ||
headers = sys.argv[1:] | ||
|
||
uname = list(os.uname()) | ||
del uname[1] | ||
uname = " ".join(uname) | ||
print(f"# used_linux_headers: {headers}") | ||
|
||
|
||
# ----------------------------------------------------------------------------- | ||
macro_regex = r"#define +((?:KEY|ABS|REL|SW|MSC|LED|BTN|REP|SND|ID|EV|BUS|SYN|FF|UI_FF|INPUT_PROP)_\w+)" | ||
macro_regex = re.compile(macro_regex) | ||
|
||
# ----------------------------------------------------------------------------- | ||
template = rf""" | ||
# Automatically generated by evdev.genecodes | ||
# Generated on {uname} | ||
# Headers: {headers} | ||
|
||
""" | ||
|
||
def parse_header(header): | ||
lines = "" | ||
for line in open(header): | ||
macro = macro_regex.search(line) | ||
if macro: | ||
#lines += f" {macro.group(1)}: int{os.linesep}" | ||
lines += f"{macro.group(1)}: int{os.linesep}" | ||
|
||
return lines | ||
|
||
|
||
for header in headers: | ||
try: | ||
fh = open(header) | ||
except (IOError, OSError): | ||
print(f"Unable to read header file: {header}") | ||
continue | ||
template += f"{parse_header(header)}" | ||
|
||
print(template) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested my suggested code yet. Anyhow
- I think it's good practice to avoid code that gets executed when importing a file. So this should be warpped in
if __name__ == '__main__':
. (I think the same way about genecodes.py, but refactoring this now is not important) - Also, I split this into a few separate specialized functions. I think that makes it a bit more readable.
I wouldn't want to bother you with somewhat opinionated review comments, so I just made the refactor myself:
#! /usr/bin/env python3 | |
""" | |
Generate a Python extension module with the constants defined in linux/input.h. | |
""" | |
from __future__ import print_function | |
import os, sys, re | |
# ----------------------------------------------------------------------------- | |
# The default header file locations to try. | |
headers = [ | |
"/usr/include/linux/input.h", | |
"/usr/include/linux/input-event-codes.h", | |
"/usr/include/linux/uinput.h", | |
] | |
if sys.argv[1:]: | |
headers = sys.argv[1:] | |
uname = list(os.uname()) | |
del uname[1] | |
uname = " ".join(uname) | |
print(f"# used_linux_headers: {headers}") | |
# ----------------------------------------------------------------------------- | |
macro_regex = r"#define +((?:KEY|ABS|REL|SW|MSC|LED|BTN|REP|SND|ID|EV|BUS|SYN|FF|UI_FF|INPUT_PROP)_\w+)" | |
macro_regex = re.compile(macro_regex) | |
# ----------------------------------------------------------------------------- | |
template = rf""" | |
# Automatically generated by evdev.genecodes | |
# Generated on {uname} | |
# Headers: {headers} | |
""" | |
def parse_header(header): | |
lines = "" | |
for line in open(header): | |
macro = macro_regex.search(line) | |
if macro: | |
#lines += f" {macro.group(1)}: int{os.linesep}" | |
lines += f"{macro.group(1)}: int{os.linesep}" | |
return lines | |
for header in headers: | |
try: | |
fh = open(header) | |
except (IOError, OSError): | |
print(f"Unable to read header file: {header}") | |
continue | |
template += f"{parse_header(header)}" | |
print(template) | |
#! /usr/bin/env python3 | |
""" | |
Generate a Python extension module with the constants defined in linux/input.h. | |
""" | |
from __future__ import print_function | |
import os | |
import sys | |
import re | |
from typing import List | |
def get_headers() -> List[str]: | |
# The default header file locations to try. | |
header_files = [ | |
"/usr/include/linux/input.h", | |
"/usr/include/linux/input-event-codes.h", | |
"/usr/include/linux/uinput.h", | |
] | |
if sys.argv[1:]: | |
header_files = sys.argv[1:] | |
print(f"# used_linux_headers: {header_files}") | |
return header_files | |
def get_uname() -> str: | |
uname = list(os.uname()) | |
del uname[1] | |
return " ".join(uname) | |
def parse_header(header_file: str) -> str: | |
macro_regex = r"#define +((?:KEY|ABS|REL|SW|MSC|LED|BTN|REP|SND|ID|EV|BUS|SYN|FF|UI_FF|INPUT_PROP)_\w+)" | |
macro_regex = re.compile(macro_regex) | |
lines = "" | |
for line in open(header_file): | |
macro = macro_regex.search(line) | |
if macro: | |
# lines += f" {macro.group(1)}: int{os.linesep}" | |
lines += f"{macro.group(1)}: int{os.linesep}" | |
return lines | |
def generate_template(uname: str, headers: List[str]) -> str: | |
template = rf""" | |
# Automatically generated by evdev.genecodes | |
# Generated on {uname} | |
# Headers: {headers} | |
""" | |
for header in headers: | |
try: | |
fh = open(header) | |
except (IOError, OSError): | |
print(f"Unable to read header file: {header}") | |
continue | |
template += f"{parse_header(header)}" | |
return template | |
if __name__ == '__main__': | |
template = generate_template( | |
get_uname(), | |
get_headers() | |
) | |
print(template) |
print("writing %s (using %s)" % (ecodes_path, " ".join(headers))) | ||
with ecodes_path.open("w") as fh: | ||
print("writing %s (using %s)" % (ecodes_c_path, " ".join(headers))) | ||
with ecodes_c_path.open("w") as fh: | ||
cmd = [sys.executable, "evdev/genecodes.py", *headers] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why this was done using the subprocess
module. Wouldn't just calling a method that does this be much cleaner?
For example in the case of the pyi stub, what would be a reason to not call generate_template
and write the result into a file, without spawning a subprocess?
Anyhow, I'm fine with doing it as a subprocess since it has already been working like that in the past.
use_stubs = True | ||
|
||
# I've been reading through setuptools docs, but i can't for the lofe of me figure out how to bring the pyi stubs into the package "cleanly" | ||
class install(_install.install): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The generation of ecodes.c doesn't seem to require such a step, why does ecodes.pyi?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ecodes.pyi
should be in the .gitignore
file, just like ecodes.c
@sezanzeb Don't worry, I've been on the lazy side "projectifying" and my thing as well. I could've refined this further during this time. I'll get back to you 😁 |
Helps your $EDITOR find the constants from ecodes.c, making for a more pleasant developer experience.
I've got a "grand plan" to extend evdevremapkeys to support importing your own Python modules, and when building those modules it would be very nice to get autocomplete and type checking for ecodes.KEY_A and such.
I'm relatively new to Python and haven't messed with setuptools before at all, I've hacked together the install class to copy the pyi file with the wheel but I'm eager to know of a better way (MANIFEST.in doesn't seem to work because the file doesn't exist when setuptools starts).