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

expand ruff rules #73

Merged
merged 11 commits into from
Mar 6, 2024
Merged

expand ruff rules #73

merged 11 commits into from
Mar 6, 2024

Conversation

barneydobson
Copy link
Collaborator

@barneydobson barneydobson commented Mar 4, 2024

Description

implement new ruff rules
Fixes #38
Also replaces I think all of os with Path, related to #9 (I guess it doesn't quite close it because the validation part needs to come in somewhere... which probably won't be closed until we deal with config #10 )

OK think I've implemented all of @cheginit 's suggestions for pre-commit, the only thing I disabled was the refurb chaining suggestions, some of them seemed OK but some places there was (at least as far as I could see) no way to do chaining without making the code a lot less readable.

@barneydobson barneydobson self-assigned this Mar 4, 2024
@barneydobson barneydobson requested review from cheginit and dalonsoa and removed request for cheginit March 5, 2024 16:58
@@ -779,7 +773,7 @@ def __call__(self, G: nx.Graph,
# Push the neighbor to the heap
heappush(heap, (alt_dist, neighbor))

edges_to_keep = set()
edges_to_keep: set = set() # no clue why mypy wants me to do this..
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe doing this will satisfy mypy: edges_to_keep: set[tuple[int, int]] = {}

Comment on lines +111 to +112
existing_input_file = Path(__file__).parent / 'defs' /\
'basic_drainage_all_bits.inp'
Copy link
Collaborator

@cheginit cheginit Mar 5, 2024

Choose a reason for hiding this comment

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

I find using Path like so Path(Path(__file__).parent, 'defs', 'basic_drainage_all_bits.inp') to be more readable. I don't like using /. But if you prefer it this way, it's ok.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm of the opposite opinion, I like / way more, so it is entirely up to you @barneydobson ! 😄

Copy link
Collaborator Author

@barneydobson barneydobson Mar 6, 2024

Choose a reason for hiding this comment

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

Inertia probably means I'll leave it as is ;)

@cheginit
Copy link
Collaborator

cheginit commented Mar 5, 2024

The rest looks good to me.

Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

Looks good to me!

swmmanywhere/post_processing.py Outdated Show resolved Hide resolved
swmmanywhere/post_processing.py Outdated Show resolved Hide resolved
barneydobson and others added 4 commits March 6, 2024 10:19
Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com>
Co-authored-by: Diego Alonso Álvarez <6095790+dalonsoa@users.noreply.github.com>
@barneydobson barneydobson merged commit ee4296a into main Mar 6, 2024
10 checks passed
@barneydobson barneydobson deleted the 38-use-more-extensive-ruff-rules branch March 6, 2024 10:32
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.

Use more extensive ruff rules
3 participants