Skip to content

Commit bfb8d77

Browse files
committed
Opt into all pydocstyle rules by default
Most of the changes here were applied automatically (and are just newlines), but a few were done by hand. In any case, it's sensible to lint into the ruleset by default, *then* ignore what we don't need (had we been doing this from the start, we would have been opted into the new & helpful rule D205 for enforcing one blank line between summary & description, per https://peps.python.org/pep-0257/#multi-line-docstrings
1 parent ccc558d commit bfb8d77

15 files changed

+24
-29
lines changed

pyproject.toml

+12-6
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ lint.select = [
99
"C4", # flake8-comprehensions
1010
"C90", # mccabe
1111
"COM", # flake8-commas
12+
"D", # pydocstyle
1213
"DJ", # flake8-django
1314
"DTZ", # flake8-datetimez
1415
"E", # pycodestyle error
@@ -46,12 +47,6 @@ lint.select = [
4647
"W", # pycodestyle warning
4748
"YTT", # flake8-2020
4849

49-
# Docstring normalization
50-
"D404", # Don't start docstrings with "This"
51-
"D200", # One-line docstrings on one line
52-
"D210", # No superfluous whitespace
53-
"D212", # Multi-line docstrings start on first line
54-
5550
# Interesting, should perhaps turn on later:
5651
# "PTH", # flake8-use-pathlib
5752
# "FLY", # flynt
@@ -82,7 +77,14 @@ lint.ignore = [
8277
"D100", # Not all public classes need docstrings.
8378
"D101", # Not all public classes need docstrings.
8479
"D102", # Not all public methods need docstrings.
80+
"D103", # Not all public methods need docstrings.
81+
"D104", # Not all packages need docstrings.
82+
"D105", # Not all magic methods need docstrings.
83+
"D106", # Not all public classes need docstrings.
8584
"D107", # Not all `__init__` methods need docstrings.
85+
"D400", # I don't care if summary lines are sentences
86+
"D401", # I don't care how summary lines are conjugated
87+
"D415", # I don't care if summary lines are sentences
8688
"N806", # Function arguments are frequently uppercase in migrations.
8789
"PLR2004", # Tests & simple logic extensively use "magic values"
8890
"PT009", # I intentionally use `unittest`-style tests.
@@ -103,6 +105,10 @@ lint.ignore = [
103105
"ISC001",
104106
"COM812", # Formatter manages commas.
105107
"E501", # Formatter manages line length.
108+
109+
# Errors that conflict with *other* rules in their own ruleset
110+
"D213", # Method summaries can go on the first line (conflicts with D212)
111+
"D203", # No blank line before class (conflicts with D211)
106112
]
107113

108114
# By default, `ruff` will respect `.gitignore`

ws/api_views.py

-3
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,6 @@ def post(self, request, *args, **kwargs):
301301
- Participant has never signed up for the trip, will be placed
302302
- Participant has signed up before, but is not on the trip
303303
"""
304-
305304
postdata = json.loads(self.request.body)
306305
par_pk = postdata.get("participant_id")
307306
notes = postdata.get("notes", "")
@@ -562,7 +561,6 @@ def dispatch(self, request, *args, **kwargs):
562561
class UpdateMembershipView(JWTView):
563562
def post(self, request, *args, **kwargs):
564563
"""Receive a message that the user's membership was updated."""
565-
566564
participant = models.Participant.from_email(self.payload["email"])
567565
if not participant: # Not in our system, nothing to do
568566
return JsonResponse({})
@@ -592,7 +590,6 @@ def post(self, request, *args, **kwargs):
592590
class OtherVerifiedEmailsView(JWTView):
593591
def get(self, request, *args, **kwargs):
594592
"""Return any other verified emails that tie to the same user."""
595-
596593
email = self.payload["email"]
597594

598595
addr = EmailAddress.objects.filter(email=email, verified=True).first()

ws/templatetags/form_tags.py

+1-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ def readonly_form(form):
1212

1313
@register.filter
1414
def instances_and_widgets(bound_field):
15-
"""Yields two-tuples of instances and widgets, designed to
16-
be used with ModelMultipleChoiceField and CheckboxSelectMultiple widgets.
15+
"""Yields instances and widgets for use with ModelMultipleChoiceField & CheckboxSelectMultiple.
1716
1817
Allows templates to loop over a multiple checkbox field and display the
1918
related model instance, such as for a table with checkboxes.

ws/templatetags/rental_tags.py

-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
@register.inclusion_tag("for_templatetags/trip_rental_table.html")
77
def trip_rental_table(trip, leader_on_trip, items_by_par, show_serial=False):
88
"""Display a table of all items rented by participants."""
9-
109
# Enforce items are only those rented before the trip itself
1110
# (Items rented by participants _after_ the trip has ended were not for the trip)
1211
items_by_par = [

ws/tests/lottery/test_graphs.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,9 @@ def test_multiple_cycles(self):
201201
def test_graph_is_a_tree(self):
202202
r"""Test when the directed graph is just a tree (has no cycles)
203203
204-
A ---> B --> C
205-
\
206-
\--> D
204+
A ---> B --> C
205+
\
206+
\--> D
207207
"""
208208
a, b, c, d = (
209209
factories.ParticipantFactory.create(name=letter) for letter in "ABCD"

ws/tests/messages/test_lottery.py

-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ def test_no_lottery_info(self):
5757
@freeze_time("2017-01-17 12:00:00 EST")
5858
def test_driver_with_no_info(self):
5959
"""We ask participants who said they could drive to supply info."""
60-
6160
par = factories.ParticipantFactory.create()
6261
factories.LotteryInfoFactory.create(participant=par, car_status="own")
6362
self.assertIsNone(par.car)

ws/tests/test_settings.py

-3
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ def _reimport_if_needed(name, *args):
4646

4747
def _fresh_settings_load(self):
4848
"""Do a fresh re-import of the settings module!"""
49-
5049
with mock.patch("builtins.__import__", side_effect=self._reimport_if_needed):
5150
reload(settings)
5251

@@ -108,7 +107,6 @@ def fake_toolbar(name, *args):
108107
def test_local_development_without_debug_toolbar(self):
109108
"""We configure installed applications properly when debug toolbar is absent.
110109
111-
112110
In most test builds, we won't have `debug_toolbar` installed, but this guarantees
113111
coverage of behavior when we do not.
114112
"""
@@ -146,7 +144,6 @@ def test_production_security(self):
146144

147145
def test_sentry_not_initialized_if_envvar_present(self):
148146
"""During local development, we can disable Sentry."""
149-
150147
with mock.patch.dict("os.environ", {"WS_DJANGO_LOCAL": "1"}, clear=True):
151148
with mock.patch.object(settings.sentry_sdk, "init") as init_sentry:
152149
self._fresh_settings_load()

ws/tests/utils/test_dates.py

-1
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ def test_past_trips(self):
236236

237237
def test_future_trips(self):
238238
"""When there are no past trips, but there are upcoming trips."""
239-
240239
self._create_ws_trip(date(2018, 1, 5))
241240
self._create_ws_trip(date(2018, 1, 6))
242241

ws/tests/utils/test_forms.py

-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ def test_normal_case(self):
1818

1919
def test_mixed_named_groups(self):
2020
"""It works mixing named groups & non-groups."""
21-
2221
choices = [
2322
(
2423
"Undergraduate student",
@@ -43,7 +42,6 @@ def test_mixed_named_groups(self):
4342

4443
def test_nested_groups(self):
4544
"""Nested named groups are handled properly."""
46-
4745
choices = [
4846
(
4947
"MIT",

ws/tests/utils/test_perms.py

-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ def setUpClass(cls) -> None:
8080

8181
def test_activity_chair(self):
8282
"""The admin can be considered an activity chair in some contexts."""
83-
8483
self.assertTrue(perm_utils.chair_or_admin(self.admin, enums.Activity.HIKING))
8584
self.assertTrue(perm_utils.is_chair(self.admin, enums.Activity.HIKING))
8685
self.assertTrue(

ws/tests/views/test_api_views.py

-1
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ def test_normal_participant(self):
163163

164164
def test_user_without_participant(self):
165165
"""We handle the case of a user who never completed a participant record."""
166-
167166
factories.UserFactory.create(email="[email protected]")
168167
response = self._query_for("[email protected]")
169168
self.assertEqual(response.status_code, 200)

ws/unsubscribe.py

+6-2
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,11 @@ def generate_unsubscribe_token(participant: models.Participant) -> str:
8181
def unsign_token(token: str) -> UnsubscribeTarget:
8282
"""Extract the participant & desired unsubscribe topics from a signed payload.
8383
84-
Raises:
84+
Raises
85+
------
8586
signing.SignatureExpired: Token is >30 days old
8687
signing.BadSignature: Token is invalid (or just >30 days old)
88+
8789
"""
8890
payload: TokenPayload = _get_signer().unsign_object(
8991
token,
@@ -108,8 +110,10 @@ def _bad_token_reason(exception: signing.BadSignature) -> str:
108110
def unsubscribe_from_token(token: str) -> models.Participant:
109111
"""Attempt to unsubscribe the participant based on an (assumed valid) token.
110112
111-
Raises:
113+
Raises
114+
------
112115
InvalidTokenError: Expired token, invalid token, or participant gone
116+
113117
"""
114118
# Any exceptions this method raises have messages meant to be consumed by humans.
115119
# We don't need the full traceback.

ws/utils/member_stats.py

-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ def fetch_geardb_stats_for_all_members(
115115
cache_strategy: CacheStrategy,
116116
) -> MemberStatsResponse:
117117
"""Report emails & rental activity for all members with current dues."""
118-
119118
acceptable_staleness = timedelta(
120119
hours=(0 if cache_strategy == CacheStrategy.BYPASS else 1)
121120
).total_seconds()

ws/utils/ratings.py

-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ def activity_enum(self) -> enums.Activity:
4040

4141
def activity_chairs(self) -> QuerySet[User]:
4242
"""Return the chairs for this activity."""
43-
4443
# It's important that this remain a method, not stored at init.
4544
# This way, views that want to get activity from self.kwargs can inherit from the mixin
4645
return perm_utils.activity_chairs(self.activity_enum)

ws/views/participant.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ def has_car(self) -> bool:
8787
return "has_car" in self.request.POST
8888

8989
def get_context_data(self, **kwargs: Any) -> dict[str, Any]:
90-
"""Return a dictionary primarily of forms to for template rendering.
90+
"""Return a dictionary primarily of forms for template rendering.
91+
9192
Also includes a value for the "I have a car" checkbox.
9293
9394
Outputs three types of forms:

0 commit comments

Comments
 (0)