Skip to content

WEB: Better management of releases #56207

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

Merged
Merged
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
15 changes: 13 additions & 2 deletions web/pandas_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import collections
import datetime
import importlib
import itertools
import json
import operator
import os
Expand All @@ -40,6 +41,7 @@
import feedparser
import jinja2
import markdown
from packaging import version
import requests
import yaml

Expand Down Expand Up @@ -239,7 +241,7 @@ def home_add_releases(context):
)
context["releases"].append(
{
"name": release["tag_name"].lstrip("v"),
"name": version.parse(release["tag_name"].lstrip("v")),
"tag": release["tag_name"],
"published": published,
"url": (
Expand All @@ -249,7 +251,16 @@ def home_add_releases(context):
),
}
)

# sorting out obsolete versions
grouped_releases = itertools.groupby(
context["releases"], key=lambda r: (r["name"].major, r["name"].minor)
)
context["releases"] = [
max(release_group, key=lambda r: r["name"].minor)
for _, release_group in grouped_releases
]
# sorting releases by version number
context["releases"].sort(key=lambda r: r["name"], reverse=True)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we sort by the parsed version? Maybe I'm missing something, but feels like sorting by name will make 2.0 be shown before 10.0, no?

Copy link
Contributor Author

@Dukastlik Dukastlik Nov 27, 2023

Choose a reason for hiding this comment

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

I'm parsing version before that on line 244. Is that ok?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, true. Makes sense, but probably worth renaming the field from name to something like parsed_version then, no? I think the code is a bit misleading otherwise, at least it was for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return context

@staticmethod
Expand Down
82 changes: 82 additions & 0 deletions web/tests/test_pandas_web.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
from unittest.mock import (
mock_open,
patch,
)

import pytest
import requests

from web.pandas_web import Preprocessors


class MockResponse:
def __init__(self, status_code: int, response: dict):
self.status_code = status_code
self._resp = response

def json(self):
return self._resp

@staticmethod
def raise_for_status():
return


@pytest.fixture
def context() -> dict:
return {
"main": {"github_repo_url": "pandas-dev/pandas"},
"target_path": "test_target_path",
}


@pytest.fixture(scope="function")
def mock_response(monkeypatch, request):
def mocked_resp(*args, **kwargs):
status_code, response = request.param
return MockResponse(status_code, response)

monkeypatch.setattr(requests, "get", mocked_resp)


_releases_list = [
{
"prerelease": False,
"published_at": "2024-01-19T03:34:05Z",
"tag_name": "v1.5.6",
"assets": None,
},
{
"prerelease": False,
"published_at": "2023-11-10T19:07:37Z",
"tag_name": "v2.1.3",
"assets": None,
},
{
"prerelease": False,
"published_at": "2023-08-30T13:24:32Z",
"tag_name": "v2.1.0",
"assets": None,
},
{
"prerelease": False,
"published_at": "2023-04-30T13:24:32Z",
"tag_name": "v2.0.0",
"assets": None,
},
{
"prerelease": True,
"published_at": "2023-01-19T03:34:05Z",
"tag_name": "v1.5.3xd",
"assets": None,
},
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can add 10.0 to make sure the possible bug commented above is not there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

]


@pytest.mark.parametrize("mock_response", [(200, _releases_list)], indirect=True)
def test_web_preprocessor_creates_releases(mock_response, context):
m = mock_open()
with patch("builtins.open", m):
context = Preprocessors.home_add_releases(context)
release_versions = [str(release["name"]) for release in context["releases"]]
assert release_versions == ["2.1.3", "2.0.0", "1.5.6"]