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

feat: add archive label and active status to ReviewModel #242

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/pyosmeta/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ class ReviewModel(BaseModel):
partners: Optional[list[Partnerships]] = None
gh_meta: Optional[GhMeta] = None
labels: list[str] = Field(default_factory=list)
active: bool = True # To indicate if package is maintained or archived

@field_validator(
"date_accepted",
Expand Down
20 changes: 19 additions & 1 deletion src/pyosmeta/models/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
from __future__ import annotations

from datetime import datetime
from enum import Enum
from typing import Any, List, Literal, Optional, Union

from pydantic import AnyUrl, BaseModel, ConfigDict, Field
from pydantic import AnyUrl, BaseModel, ConfigDict, Field, model_validator


class User(BaseModel):
Expand Down Expand Up @@ -61,6 +62,12 @@
class Owner(User): ...


class LabelType(str, Enum):
ARCHIVED = "archived"
PYOS_APPROVED = "6/pyOS-approved"
JOSS_APPROVED = "9/joss-approved"


class Labels(BaseModel):
id: Optional[int] = None
node_id: Optional[str] = None
Expand All @@ -69,6 +76,17 @@
description: Optional[str] = None
color: Optional[str] = None
default: Optional[bool] = None
type: Optional[LabelType] = None

@model_validator(mode="before")
Copy link
Member

Choose a reason for hiding this comment

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

It's been so long since i've dug into this code. let's flesh out the docstrings while this is still fresh.

can you please add a second "paragraph" that better describes what we are actually parsing here and why as a reminder (for me an our future selves)?

Copy link
Author

Choose a reason for hiding this comment

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

I clarified the docstrings in b9868eb

def parse_label_type(cls, data):
"""Parse the label type from the name before validation."""
if "name" in data:
try:
data["type"] = LabelType(data["name"])
except ValueError:
pass

Check warning on line 88 in src/pyosmeta/models/github.py

View check run for this annotation

Codecov / codecov/patch

src/pyosmeta/models/github.py#L87-L88

Added lines #L87 - L88 were not covered by tests
return data


class Issue(BaseModel):
Expand Down
22 changes: 21 additions & 1 deletion src/pyosmeta/parse_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from pydantic import ValidationError

from pyosmeta.models import ReviewModel, ReviewUser
from pyosmeta.models.github import Issue
from pyosmeta.models.github import Issue, Labels, LabelType

from .github_api import GitHubAPI
from .utils_clean import clean_date_accepted_key
Expand Down Expand Up @@ -221,6 +221,25 @@

return meta

def _postprocess_labels(self, meta: dict) -> dict:
"""
Handle specific labels that are model properties
Copy link
Member

Choose a reason for hiding this comment

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

please be more specific with the docstrings. Essentially this package can be hard to "dig back into" so let's be specific about what this helper does since we don't have great docs for the package yet.

Let's be specific about it's purpose in the docstring

Copy link
Author

Choose a reason for hiding this comment

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

I clarified the docstrings in b9868eb

"""

def _is_archived(label: str) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

this is a super explicit name but i think for the purposes of documenting process, over-documenting via docstrings is a good idea here. so let's add numpy docstrings with very clear descriptions of intent to each of these.

Copy link
Author

Choose a reason for hiding this comment

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

I added a docstring in b9868eb

if isinstance(label, str):
return "archived" in label.lower()
elif isinstance(label, Labels):
return label.type == LabelType.ARCHIVED
raise ValueError("Invalid label type")

Check warning on line 234 in src/pyosmeta/parse_issues.py

View check run for this annotation

Codecov / codecov/patch

src/pyosmeta/parse_issues.py#L234

Added line #L234 was not covered by tests

# Check if the review has the "archived" label
if "labels" in meta and [
label for label in meta["labels"] if _is_archived(label)
]:
meta["active"] = False
return meta

def _parse_field(self, key: str, val: str) -> Any:
"""
Method dispatcher for parsing specific header fields.
Expand Down Expand Up @@ -277,6 +296,7 @@

# Finalize review model before casting
model = self._postprocess_meta(model, body)
model = self._postprocess_labels(model)

return ReviewModel(**model)

Expand Down
17 changes: 17 additions & 0 deletions tests/integration/test_parse_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,20 @@ def test_parse_labels(issue_list, process_issues):
issue.labels = labels
review = process_issues.parse_issue(issue)
assert review.labels == ["6/pyOS-approved", "another_label"]
assert review.active

# Now add an archive label
label_inst = Labels(
id=1196238794,
node_id="MDU6TGFiZWwxMTk2MjM4Nzk0",
Copy link
Member

Choose a reason for hiding this comment

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

Codecov is noticing some test gaps - can you please ensure that we have full coverage via this pr?

Copy link
Author

Choose a reason for hiding this comment

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

Full coverage achieved ✅

url="https://api.github.com/repos/pyOpenSci/software-submission/labels/archived",
name="archived",
description="",
color="006B75",
default=False,
)
labels = [label_inst, "another_label"]
for issue in issue_list:
issue.labels = labels
review = process_issues.parse_issue(issue)
assert not review.active
Loading