-
Notifications
You must be signed in to change notification settings - Fork 19
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
6206754
b9868eb
ff807b9
ed443bc
8770f49
1266995
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -221,6 +221,25 @@ | |
|
||
return meta | ||
|
||
def _postprocess_labels(self, meta: dict) -> dict: | ||
""" | ||
Handle specific labels that are model properties | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I clarified the docstrings in b9868eb |
||
""" | ||
|
||
def _is_archived(label: str) -> bool: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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. | ||
|
@@ -277,6 +296,7 @@ | |
|
||
# Finalize review model before casting | ||
model = self._postprocess_meta(model, body) | ||
model = self._postprocess_labels(model) | ||
|
||
return ReviewModel(**model) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
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