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

Turn on and fix Python linting of examples #1191

Merged
merged 21 commits into from
Feb 11, 2023
Merged

Turn on and fix Python linting of examples #1191

merged 21 commits into from
Feb 11, 2023

Conversation

emilk
Copy link
Member

@emilk emilk commented Feb 10, 2023

Mostly superficial fixes, but one important:

- except:
+ except Exception:

The first also catches KeyboardInterrupt, which is quite annoying when you are trying to Ctrl-C an example.

TODO:

  • Run just py-run-all

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I've added a line to CHANGELOG.md (if this is a big enough change to warrant it)

@emilk emilk changed the title Turn on python linting of examples Turn on and fix Python linting of examples Feb 10, 2023
@emilk emilk marked this pull request as ready for review February 10, 2023 22:58
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

TIL string interpolation in python can't do multiline

examples/dicom/main.py Outdated Show resolved Hide resolved
@@ -85,7 +85,7 @@ def download_and_extract(url: str, path: Path) -> None:
def download_glb_sample(name: str) -> Path:
"""Downloads a sample glb mesh and returns the path it was downloaded to."""
capitalized_name = name.capitalize()
url = f"https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/{capitalized_name}/glTF-Binary/{capitalized_name}.glb?raw=true"
url = f"https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/{capitalized_name}/glTF-Binary/{capitalized_name}.glb?raw=true" # noqa: E501 line too long
Copy link
Member

Choose a reason for hiding this comment

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

pep8 says https://stackoverflow.com/a/54950733

Suggested change
url = f"https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/{capitalized_name}/glTF-Binary/{capitalized_name}.glb?raw=true" # noqa: E501 line too long
url = (
"https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/"
f"{capitalized_name}/glTF-Binary/{capitalized_name}.glb?raw=true"
)

Copy link
Member Author

Choose a reason for hiding this comment

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

no, didn't work

@@ -20,14 +18,14 @@ def task(title: str) -> None:

rr.log_text_entry(
"log",
text=f"Logging from pid={os.getpid()}, thread={threading.get_ident()} using the rerun recording id {rr.get_recording_id()}",
text=f"Logging from pid={os.getpid()}, thread={threading.get_ident()} using the rerun recording id {rr.get_recording_id()}", # noqa: E501 line too long
Copy link
Member

@Wumpf Wumpf Feb 11, 2023

Choose a reason for hiding this comment

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

Suggested change
text=f"Logging from pid={os.getpid()}, thread={threading.get_ident()} using the rerun recording id {rr.get_recording_id()}", # noqa: E501 line too long
text=(
f"Logging from pid={os.getpid()}, thread={threading.get_ident()} "
f"using the rerun recording id {rr.get_recording_id()}",
)

Copy link
Member Author

Choose a reason for hiding this comment

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

nope, didn't work

'Wasm',
'num_instances',
'instances_count',
"if let Err(err) = foo",
Copy link
Member

Choose a reason for hiding this comment

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

but why are the above still in ticks?

Copy link
Member Author

Choose a reason for hiding this comment

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

because they contain "

@emilk emilk force-pushed the emilk/lint-examples branch from 2825069 to e230757 Compare February 11, 2023 09:52
@emilk emilk merged commit 49cea4c into main Feb 11, 2023
@emilk emilk deleted the emilk/lint-examples branch February 11, 2023 10:11
emilk added a commit that referenced this pull request Feb 11, 2023
* Turn on python linting of examples

* Fix minor lints

* Mute lints about bad docstrings

* Use list() instead of trivial list comprehension

* ignore some lines, fix others

* don't use single-letter names

* ignore line

* `catch:` -> `catch Exception:`

* Dict comprehension

* format

* Docstring improvements

* More docstring fixes

* Docstring fixes

* docstrings done!

* Enable and fix more lints

* One more

* Clean up .flake8 file

* Final cleanup of .flake8

* Also lint python scripts under scripts/

* py-format

* better dicom explanation

Co-authored-by: Andreas Reich <[email protected]>

---------

Co-authored-by: Andreas Reich <[email protected]>
emilk added a commit that referenced this pull request Feb 11, 2023
* Split python and rust examples into separate folders

* objectron: ensure logging reaches the screen

* try fix

* remove dead shellscript

* fix objectron build-script

* Fixes

* Fix Rust objectron demo

* Add readme for rust examples

* lint

* fix mypy

* Improve the docs of `rerun` (#1179)

* Split out the `run` parts of the rerun library to its own file

* Document that the web feature flag requires running scripts/setup_web.sh

* Improve the docs of rerun and re_sdk

* Describe more things

* Fix warning

* More docs, and split out `coordinates` into its own module

* nit comment

Co-authored-by: Andreas Reich <[email protected]>

* Improve docstring

* Improve error message

* Add some spacing

* For instance -> For example

---------

Co-authored-by: Andreas Reich <[email protected]>

* Add newlines before types, `impl`, `fn`, and docstrings

* Update examples/requirements.txt

* Mark git directories as safe for our docker_ci container and fix cargo caching (#1187)

* Mark git directories as safe for our docker_ci container
* New cargo packages means a new cache key
* Add CACHE_KEY everywhere and Only update cache from 1 location

* Run cargo check after patching pre-release (#1190)

* Allow logging the contents of an image file, not by path (#1193)

* log_image_file: require named arguments
* Allow logging the contents of an image file, not by path

* Add --dataset and --resize flags to colmap (#1177)

* Add --dataset and --resize flags to colmap

* Enable `--web-viewer` in the Python Wheels (#1182)

* Make 'web' a default feature of the python sdk

* Add sanity-checking to crates/re_web_server/build.rs

* Allow crates/re_web_server/build.rs to be run from any CWD

* Run setup_web.sh on the Python CI

* Turn on the web feature on the CI

* Only enable the web feature when building pypi package

* Improve setup_web.sh

* Add winget package manager to setup_web.sh

* Print out what is happening

* Add shell: bash

* Try getting wasm-opt into the windows build

* Correct path to exe

* run setup_web.sh but allow it to fail

---------

Co-authored-by: Jeremy Leibs <[email protected]>

* Install web dependencies before running cargo check (#1196)

* Update arrow2, arrow2_convert and polars (#1189)

* Update arrow2, arrow2_convert and polars

* Patch arrow2-convert again

* Add instructions for how to patch crates to Cargo.toml

* Use trunk polars in patch

* Turn on and fix Python linting of examples (#1191)

* Turn on python linting of examples

* Fix minor lints

* Mute lints about bad docstrings

* Use list() instead of trivial list comprehension

* ignore some lines, fix others

* don't use single-letter names

* ignore line

* `catch:` -> `catch Exception:`

* Dict comprehension

* format

* Docstring improvements

* More docstring fixes

* Docstring fixes

* docstrings done!

* Enable and fix more lints

* One more

* Clean up .flake8 file

* Final cleanup of .flake8

* Also lint python scripts under scripts/

* py-format

* better dicom explanation

Co-authored-by: Andreas Reich <[email protected]>

---------

Co-authored-by: Andreas Reich <[email protected]>

* Update the paths in .flake8

* examples/deep_sdf: sort scenes by relevance instead of lexically + better default one

* examples/raw_mesh: sort scenes by relevance instead of lexically + better default one

* examples/raw_mesh: now offering a higher level --scene flag

---------

Co-authored-by: Clement Rey <[email protected]>
Co-authored-by: Andreas Reich <[email protected]>
Co-authored-by: Jeremy Leibs <[email protected]>
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