Skip to content

Upgrade tox and linter to conventions used by other projects#831

Merged
f4str merged 10 commits intomainfrom
server/tox-upgrade
Jul 13, 2021
Merged

Upgrade tox and linter to conventions used by other projects#831
f4str merged 10 commits intomainfrom
server/tox-upgrade

Conversation

@f4str
Copy link

@f4str f4str commented Jul 8, 2021

Updated tox to use flake8 for the linter and changed black and isort configurations to match linting and styling conventions used by all other projects for consistency (most noticeably changing line-length from 88 to 100).

mypy's newest release allows configurations to be stored in pyproject.toml instead of requiring its own mypy.ini.

Cleaned up the setup.py dev requirements as they are no longer needed since tox will automatically install them in a virtualenv wrapper when needed.

All changes made are purely linting fixes (most of which are by black and isort auto-formatting) such as:

  • removing unused imports
  • removing unused variables
  • docstring capitalization/formatting
  • changing type annotations
  • consolidating/expanding code into less/more lines

@f4str f4str changed the title Upgrade tox and liner to conventions used by other projects Upgrade tox and linter to conventions used by other projects Jul 8, 2021
@f4str f4str marked this pull request as ready for review July 9, 2021 15:13
@f4str f4str requested review from BryonLewis and subdavis July 9, 2021 15:23
use_parentheses = true
include_trailing_comma = true
multi_line_output = 3
profile = "black"
Copy link
Contributor

Choose a reason for hiding this comment

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

This was like the greatest addition to isort ever.

Copy link
Contributor

@subdavis subdavis left a comment

Choose a reason for hiding this comment

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

Please fix string literal line wraps. Everything else looks good!

Comment on lines 16 to 17
"2,3.png,2,10,50,20,35,1,-1,type3,0.765,(kp) head 22.4534 45.6564,"
"(kp) tail 55.232 22.3445",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is OK style. Could you either come up with a way to let these string literals span their full width? I don't even care if you have to disable this rule for this file, this makes the code harder to read IMO.

Copy link
Collaborator

@BryonLewis BryonLewis Jul 13, 2021

Choose a reason for hiding this comment

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

should the content for these tests be moved to ./testutils and loaded in like for the image-sort. Then you no longer are worrying about linting and formatting test data? You may lose the in-line comments though.

Copy link
Contributor

Choose a reason for hiding this comment

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

They're paired input/expected, so you'd have to organize them as such.

I don't feel that strongly about it, but even if you moved these, you still couldn't have str literals longer than 100chars, which as a concept I would like to resolve.

Comment on lines 276 to 293
"0,1.png,0,884.66,510,1219.66,737.66,1,-1,typestring,1,(atr) DetectionNumber 2.002,"
"(atr) DetectionPredefinedValue value1",
"0,2.png,1,111,222,3333,444,1,-1,typestring,1,(trk-atr) booleanAttr true,"
"(atr) DetectionPredefinedValue value2",
"0,3.png,2,747,457,1039,633,1,-1,typestring,1,(trk-atr) booleanAttr true,"
"(atr) DetectionPredefinedValue value3",
"0,3.png,3,884.66,510,1219.66,737.66,1,-1,typestring,1,(trk-atr) booleanAttr true,"
"(atr) DetectionPredefinedValue value1",
"0,4.png,4,111,222,3333,444,1,-1,typestring,1,(atr) DetectionPredefinedValue value2",
"0,5.png,5,747,457,1039,633,1,-1,typestring,1,(atr) DetectionPredefinedValue value3",
"1,1.png,0,884.66,510,1219.66,737.66,1,-1,typestring,1,(atr) DetectionNumber 2.002,(atr) DetectionPredefinedValue value1",
"1,2.png,1,111,222,3333,444,1,-1,typestring,1,(trk-atr) booleanAttr true,(atr) DetectionPredefinedValue value2",
"1,3.png,2,747,457,1039,633,1,-1,typestring,1,(trk-atr) booleanAttr true,(atr) DetectionPredefinedValue value3",
"1,3.png,3,884.66,510,1219.66,737.66,1,-1,typestring,1,(trk-atr) booleanAttr true,(atr) DetectionPredefinedValue value1",
"1,1.png,0,884.66,510,1219.66,737.66,1,-1,typestring,1,(atr) DetectionNumber 2.002,"
"(atr) DetectionPredefinedValue value1",
"1,2.png,1,111,222,3333,444,1,-1,typestring,1,(trk-atr) booleanAttr true,"
"(atr) DetectionPredefinedValue value2",
"1,3.png,2,747,457,1039,633,1,-1,typestring,1,(trk-atr) booleanAttr true,"
"(atr) DetectionPredefinedValue value3",
"1,3.png,3,884.66,510,1219.66,737.66,1,-1,typestring,1,(trk-atr) booleanAttr true,"
"(atr) DetectionPredefinedValue value1",
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Comment on lines 25 to 26
"5,6.png,6,10,10,20,20,1,-1,type1,0.89,(atr) attrNAME spaced attr name,"
"(trk-atr) booleanAttr true",
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Comment on lines 200 to 203
"0,1.png,0,884,510,1219,737,1.0,-1,typestring,1.0,(atr) detectionAttr frame 0 attr,"
"(trk-atr) trackATTR TestTrack ATTR With Space",
"0,2.png,1,111,222,3333,444,1.0,-1,typestring,1.0,(atr) detectionAttr frame 1 attr,"
"(trk-atr) trackATTR TestTrack ATTR With Space",
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@f4str
Copy link
Author

f4str commented Jul 13, 2021

Fixed the string wrapping and added the flake8 config to ignore line length linting for tests/

Also added the tox -e docs option to run mkdocs and serve the site. Thus mkdocs was removed from the dev requirements since tox will automatically install it. This is documented in server/README.md

Finally changed the version number in server/setup.py to match client/package.json.

@f4str f4str merged commit bae0650 into main Jul 13, 2021
@f4str f4str deleted the server/tox-upgrade branch July 13, 2021 18:23
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.

3 participants