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

fix: make sure eic roles are populated correctly #235

Merged
merged 2 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

### Fixed

* Fix: Eix field not processing correctly (@lwasser, #234)
* Fix: Updated documentation throughout with a focus on how a user's name is accessed and updated (@lwasser)
* Fix: ReviewUser object name can be optional. There are times when we don't have the actual person's name only the GH username (@lwasser)

Expand Down
35 changes: 20 additions & 15 deletions src/pyosmeta/cli/update_review_teams.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,23 +148,28 @@ def main():
for role in contrib_types.keys():
user: list[ReviewUser] | ReviewUser = getattr(review, role)

# Handle lists or single users separately
if isinstance(user, list):
for i, a_user in enumerate(user):
a_user, contribs = process_user(
a_user, role, pkg_name, contribs, process_contribs
# Eic is a newer field, so in some instances it will be empty
# if it's empty print a message noting the data are missing
if user:
# Handle lists or single users separately
if isinstance(user, list):
for i, a_user in enumerate(user):
a_user, contribs = process_user(
a_user, role, pkg_name, contribs, process_contribs
)
# Update individual user in reference to issue list
user[i] = a_user
elif isinstance(user, ReviewUser):
user, contribs = process_user(
user, role, pkg_name, contribs, process_contribs
)
setattr(review, role, user)
else:
raise TypeError(
"Keys in the `contrib_types` map must be a `ReviewUser` or `list[ReviewUser]` in the `ReviewModel`"
)
# Update individual user within the user list
user[i] = a_user
elif isinstance(user, ReviewUser):
user, contribs = process_user(
user, role, pkg_name, contribs, process_contribs
)
setattr(review, role, user)
else:
raise TypeError(
"Keys in the `contrib_types` map must be a `ReviewUser` or `list[ReviewUser]` in the `ReviewModel`"
)
print(f"I can't find a username for {role}. Moving on.")

# Export to yaml
contribs_ls = [model.model_dump() for model in contribs.values()]
Expand Down
13 changes: 13 additions & 0 deletions src/pyosmeta/contributors.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,20 @@ def __init__(self, github_api: GitHubAPI, json_files: List) -> None:
"github_username",
]

"""A dictionary that maps a category type found in our peer review
submission template to the roles and role categories that a contributor
should have populated associated with the review in the contributors.yml

example
reviewers is in the software submission template like this:
reviewers: @username, @username2
Being a reviewer maps to two roles in the users contributions to
pyOpenSci: reviewer and peer-review
packages_reviewed is another item in the contributors file that lists
all of the packages that user has reviewed. So if they ar e
"""
self.contrib_types = {
"eic": ["packages_eic", ["eic", "peer-review"]],
"reviewers": ["packages_reviewed", ["reviewer", "peer-review"]],
"editor": ["packages_editor", ["editor", "peer-review"]],
"submitting_author": [
Expand Down
2 changes: 0 additions & 2 deletions src/pyosmeta/file_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,6 @@ def export_yaml(filename: str, data_list: list):
yaml.dump(data_list, file)


# TODO: Double check - i may be able to combine this with the other clean
# function created
def clean_string(astr: str) -> str:
"""
Clean - remove strings starting with "*id0" and "[]".
Expand Down
3 changes: 3 additions & 0 deletions src/pyosmeta/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ class PersonModel(BaseModel, UrlValidatorMixin):
)
board: Optional[bool] = False
contributor_type: Set[str] = set()
packages_eic: Set[str] = set()
packages_editor: Set[str] = set()
packages_submitted: Set[str] = set()
packages_reviewed: Set[str] = set()
Expand All @@ -143,6 +144,7 @@ def validate_bool_fields(cls, v: Any) -> bool:
return False

@field_validator(
"packages_eic",
"packages_reviewed",
"packages_submitted",
"packages_editor",
Expand Down Expand Up @@ -176,6 +178,7 @@ def add_unique_value(self, attr_name: str, values: Union[str, list[str]]):
raise ValueError(f"{attr_name} is not a set attribute")

@field_serializer(
"packages_eic",
"packages_reviewed",
"packages_submitted",
"packages_editor",
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

@pytest.fixture
def ghuser_response():
"""This is the initial github response. I changed the username to
"""This is the initial GitHub response. I changed the username to
create this object"""
expected_response = {
"login": "chayadecacao",
Expand Down
59 changes: 33 additions & 26 deletions tests/integration/test_review_model.py
Original file line number Diff line number Diff line change
@@ -1,36 +1,43 @@
import pytest

from pyosmeta.models import ReviewModel


# We could setup some example data using fixtures and a conf.py
# Once we have a better view of the test suite.
example = {
"submitting_author": {
"github_username": "nabobalis",
"name": "Nabil Freij",
},
"all_current_maintainers": [],
"package_name": "sunpy",
"one-line_description_of_package": "Python for Solar Physics",
"repository_link": "https://github.com/sunpy/sunpy",
"version_submitted": "5.0.1",
"editor": {"github_username": "cmarmo", "name": ""},
"reviewer_1": {"github_username": "Septaris", "name": ""},
"reviewer_2": {"github_username": "nutjob4life", "name": ""},
"archive": "[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.8384174.svg)](https://doi.org/10.5281/zenodo.8384174)",
"version_accepted": "5.1.1",
"joss_doi": "[![DOI](https://joss.theoj.org/papers/10.21105/joss.01832/status.svg)](https://joss.theoj.org/papers/10.21105/joss.01832)",
"date_accepted": "01/18/2024",
"categories": [
"data-retrieval",
"data-extraction",
"data-processing/munging",
"data-visualization",
],
}
@pytest.fixture
def review_data():
return {
"submitting_author": {
"github_username": "nabobalis",
"name": "Nabil Freij",
},
"all_current_maintainers": [],
"package_name": "sunpy",
"one-line_description_of_package": "Python for Solar Physics",
"repository_link": "https://github.com/sunpy/sunpy",
"version_submitted": "5.0.1",
"eic": {"github_username": "cmarmo", "name": ""},
"editor": {"github_username": "cmarmo", "name": ""},
"reviewer_1": {"github_username": "Septaris", "name": ""},
"reviewer_2": {"github_username": "nutjob4life", "name": ""},
"archive": "[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.8384174.svg)](https://doi.org/10.5281/zenodo.8384174)",
"version_accepted": "5.1.1",
"joss_doi": "[![DOI](https://joss.theoj.org/papers/10.21105/joss.01832/status.svg)](https://joss.theoj.org/papers/10.21105/joss.01832)",
"date_accepted": "01/18/2024",
"categories": [
"data-retrieval",
"data-extraction",
"data-processing/munging",
"data-visualization",
],
}


def test_alias_choices_validation():
def test_alias_choices_validation(review_data):
"""Test that model correctly recognizes the field alias"""

new = ReviewModel(**example)
new = ReviewModel(**review_data)
assert new.date_accepted == "2024-01-18"
assert new.package_description == "Python for Solar Physics"
assert new.eic.github_username == "cmarmo"
6 changes: 3 additions & 3 deletions tests/unit/test_github_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,21 +147,21 @@ def test_api_endpoint_with_invalid_dates(after_date, expected_url):
def test_get_user_info_successful(mocker, ghuser_response):
"""Test that an expected response returns properly"""

expected_response = ghuser_response
mock_response = mocker.Mock()
mock_response.status_code = 200
mock_response.json.return_value = expected_response
mock_response.json.return_value = ghuser_response
mocker.patch("requests.get", return_value=mock_response)

github_api_instance = GitHubAPI()
user_info = github_api_instance.get_user_info("example_user")

assert user_info == expected_response
assert user_info == ghuser_response


def test_get_user_info_bad_credentials(mocker):
"""Test that a value error is raised when the GH token is not
valid."""

mock_response = mocker.Mock()
mock_response.status_code = 401
mocker.patch("requests.get", return_value=mock_response)
Expand Down
9 changes: 0 additions & 9 deletions tests/unit/test_update_review_teams.py

This file was deleted.