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

Formatting changes to enable more Ruff Rules #214

Merged
merged 31 commits into from
Mar 14, 2024
Merged

Conversation

jaredsnyder
Copy link
Contributor

@jaredsnyder jaredsnyder commented Feb 29, 2024

fixes #213

@jaredsnyder jaredsnyder changed the title Formatting changes to enable more Ruff Rules (WIP) Formatting changes to enable more Ruff Rules Feb 29, 2024
@jaredsnyder
Copy link
Contributor Author

Ruff does not yet support Sphinx-style docstrings 🙃 astral-sh/ruff#6606

@jaredsnyder
Copy link
Contributor Author

Removed the docstring check because sphinx isn't yet supported. Left the updates though because when sphinx is supported it'll be less work to modify them than to do it over again

@codecov-commenter
Copy link

codecov-commenter commented Feb 29, 2024

Codecov Report

Attention: Patch coverage is 77.02703% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 78.84%. Comparing base (2722331) to head (a3bd6db).

Files Patch % Lines
src/mozanalysis/experiment.py 63.63% 8 Missing ⚠️
src/mozanalysis/sizing.py 78.57% 3 Missing ⚠️
src/mozanalysis/frequentist_stats/sample_size.py 80.00% 2 Missing ⚠️
src/mozanalysis/bayesian_stats/binary.py 0.00% 1 Missing ⚠️
src/mozanalysis/bq.py 0.00% 1 Missing ⚠️
src/mozanalysis/frequentist_stats/bootstrap.py 50.00% 1 Missing ⚠️
src/mozanalysis/metrics/__init__.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #214      +/-   ##
==========================================
- Coverage   79.38%   78.84%   -0.54%     
==========================================
  Files          23       23              
  Lines        1033     1035       +2     
==========================================
- Hits          820      816       -4     
- Misses        213      219       +6     
Flag Coverage Δ
project 78.84% <77.02%> (-0.54%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jaredsnyder
Copy link
Contributor Author

reverted the docstring stuff because sphinx doesn't like it :(

@jaredsnyder jaredsnyder changed the title (WIP) Formatting changes to enable more Ruff Rules Formatting changes to enable more Ruff Rules Feb 29, 2024
ruff.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@mikewilli mikewilli 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! Mostly converting .format() strings to f-strings, and sorting imports, but still nice to have.

Might be nice to get some DS eyes on this since this is mostly a DS codebase, but I'll leave that up to you. Thanks!

src/mozanalysis/metrics/__init__.py Show resolved Hide resolved
tests/bayesian_stats/test_survival_func.py Show resolved Hide resolved
ruff.toml Show resolved Hide resolved
setup.py Show resolved Hide resolved
src/mozanalysis/config.py Show resolved Hide resolved
Copy link
Contributor

@danielkberry danielkberry left a comment

Choose a reason for hiding this comment

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

LGTM

@jaredsnyder jaredsnyder merged commit c70d77b into main Mar 14, 2024
3 checks passed
@jaredsnyder jaredsnyder deleted the formatting_changes branch March 14, 2024 14:55
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.

Add more checks to Ruff for cleaner code
4 participants