-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add pylint configuration, fix linting issues, and update CI pipeline for Python #467
Add pylint configuration, fix linting issues, and update CI pipeline for Python #467
Conversation
a58c285
to
377f8c1
Compare
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.
Good stuff so far! Only minor spelling and gotta agree on input output typing of functions/methods in python. Should be consistent. Maybe too advanced for linters to autofix, but they could at least detect it?
acoustics/acoustics_data_record/acoustics_data_record/acoustics_data_record_node.py
Outdated
Show resolved
Hide resolved
acoustics/acoustics_data_record/acoustics_data_record/acoustics_data_record_node.py
Outdated
Show resolved
Hide resolved
acoustics/acoustics_data_record/acoustics_data_record/acoustics_data_record_node.py
Outdated
Show resolved
Hide resolved
acoustics/acoustics_data_record/acoustics_data_record/acoustics_data_record_node.py
Outdated
Show resolved
Hide resolved
self.get_logger().info("Acoustics PCB MCU IP: 10.0.0.111") | ||
self.get_logger().info("Trying to connect...") | ||
|
||
TeensyCommunicationUDP.init_communication(frequenciesOfInterest) | ||
TeensyCommunicationUDP.init_communication(frequencies_of_interest) | ||
|
||
self.get_logger().info("Sucsefully connected to Acoustics PCB MCU :D") | ||
|
||
def data_update(self) -> None: |
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.
Sudden use of explicit output type. Should be consistent
|
||
This method calls the fetch_data method from the TeensyCommunicationUDP class | ||
to update the data. | ||
""" | ||
TeensyCommunicationUDP.fetch_data() | ||
|
||
def data_publisher(self) -> None: |
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.
inconsistent use of -> None
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.
Now all functions should have a return type annotation (so all functions that dont return anything now have a -> None)
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.
Closing in
acoustics/acoustics_interface/acoustics_interface/acoustics_interface_lib.py
Show resolved
Hide resolved
Just a head's up @kluge7 @Hallfred, there are a lot of deprecated typing. aliases, so using built-in types is now mostly favored (like Dict and List being deprecated for dict and list), check out https://docs.python.org/3/library/typing.html (and the Aliases to built-in types section in particular) |
This PR includes several improvements and refactorings related to code quality, linting, and CI pipeline updates. Please review the code carefully to ensure nothing broke during these changes.