Skip to content
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

Tools/LogAnalyzer: fix and update loganalyzer #21172

Merged
merged 4 commits into from
Jul 19, 2022

Conversation

mentonin
Copy link
Contributor

This PR updates LogAnalyzer, removing deprecated code, fixing incompatibilities with Python 3. It also formats the files with black.

@peterbarker
Copy link
Contributor

Do we really need to change every instance of ' to "?! What's the reasoning?

If this is flake8-clean afterwards we should label it as such so it stays that way with AP_FLAKE8_CLEAN in a comment near the top.

This script has to continue to work with Python2.

@rmackay9
Copy link
Contributor

Nice to some the log analyser getting some love!

@timtuxworth
Copy link
Contributor

timtuxworth commented Jul 13, 2022

This script has to continue to work with Python2.

Why? Python2 sunsetted more than 2 years ago? https://www.python.org/doc/sunset-python-2/

@mentonin
Copy link
Contributor Author

mentonin commented Jul 13, 2022

Do we really need to change every instance of ' to "?! What's the reasoning?

I ran it through black, because pyproject.toml makes me think it is the standard formatter for the project. According to The Black code style:

The main reason to standardize on a single form of quotes is aesthetics. Having one kind of quotes everywhere reduces reader distraction. It will also enable a future version of Black to merge consecutive string literals that ended up on the same line (see #26 for details).


If this is flake8-clean afterwards we should label it as such so it stays that way with AP_FLAKE8_CLEAN in a comment near the top.

I rechecked and there are only a few violations of line length on comments and docstrings (it looks like I had E501 disabled in a config file somewhere). Will fix. Should I label it in the file docstring or in a comment (existing code has examples of both styles).


This script has to continue to work with Python2.

I can try to fix it, but is it really necessary? Python2 has been deprecated for a while now, and supporting both is going to be increasingly complex as time goes on.

@mentonin mentonin force-pushed the Update-LogAnalyzer branch 2 times, most recently from 73e8500 to b49c31e Compare July 18, 2022 13:54
@peterbarker peterbarker merged commit b59a214 into ArduPilot:master Jul 19, 2022
@peterbarker
Copy link
Contributor

Merged, thanks!

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.

None yet

4 participants