Skip to content

GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database - #35735

Merged
jorisvandenbossche merged 4 commits into
apache:mainfrom
jorisvandenbossche:GH-35040-fix-ci
Jun 6, 2023
Merged

GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database#35735
jorisvandenbossche merged 4 commits into
apache:mainfrom
jorisvandenbossche:GH-35040-fix-ci

Conversation

@jorisvandenbossche

@jorisvandenbosschejorisvandenbossche commented May 24, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Rationale for this change

Fix up of #35395, skipping one of the tests added in that PR on Windows, because the test requires access to a tz database.

@github-actions

github-actionsBot commented May 24, 2023

Copy link
Copy Markdown

@jorisvandenbossche

jorisvandenbossche commented May 24, 2023

Copy link
Copy Markdown
MemberAuthor

@github-actions crossbow submit -g wheel

@jorisvandenbosschejorisvandenbossche mentioned this pull request May 24, 2023
Merged
@github-actions

github-actionsBot commented May 24, 2023

Copy link
Copy Markdown

Revision: 64c0d83

Submitted crossbow builds: ursacomputing/crossbow @ actions-fe7da097ff

TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions

@pitrou
pitrou requested a review from rokMay 24, 2023 10:33
@pitrou

pitrou commented May 24, 2023

Copy link
Copy Markdown
Member

@rok does this look right? We are able to somehow use a timezone database on Windows, IIRC?

@jorisvandenbossche

jorisvandenbossche commented May 24, 2023•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Yeah, the better fix would actually be to detect if the database is available, and only skip if not found on windows. Because this patch skips the tests altogether, even when it can actually work (eg on appveyor the tests pass because we download the tzdata during appveyor setup). But just to note: we currently do this plain skip for other tests requiring the database as well, so that is an existing issue with our test setup.

I assume checking if the path %USERPROFILE%\Downloads\tzdata exists might be a sufficient check at the moment (since that's the place where tz.cpp will look, and we currently don't yet allow customizing that in pyarrow).

@danepitkin

danepitkin commented May 24, 2023

Copy link
Copy Markdown
Member

Thanks for fixing! There are other tests in this file that also use pytest.importorskip("pytz"), should they be updated as well? It's not clear to me why this test case behaves differently.

rok
rok reviewed May 24, 2023
View reviewed changes

@rokrok left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For completeness - I believe db download for windows was added by @wjones127 in #12536.

Comment threadpython/pyarrow/tests/test_scalars.py Outdated
@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels May 24, 2023
@wjones127

wjones127 commented May 25, 2023

Copy link
Copy Markdown
Member

For completeness - I believe db download for windows was added by @wjones127 in #12536.

That was just for C++ and R. I don't think we figured out how to do it for Python, since we need the text form of the IANA database while Python's timezone packages provide the binary form.

raulcd
raulcd approved these changes May 27, 2023
View reviewed changes

@raulcdraulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We already have 3 tests being skipped on Windows for the same reason.
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L1851
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
I am +1 to merge

@github-actionsgithub-actionsBot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels May 27, 2023
rok
rok approved these changes May 27, 2023
View reviewed changes
jorisvandenbossche
jorisvandenbossche commented May 31, 2023
View reviewed changes
Comment threadpython/pyarrow/tests/test_compute.py
Comment on lines +1862 to +1864
if sys.platform != "win32":
# Locale-dependent formats don't match on Windows
formats.extend(["%c", "%x", "%X"])

@jorisvandenbosschejorisvandenbosscheMay 31, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The error on Appveyor we got was:

 @pytest.mark.skipif(sys.platform == "win32" and not util.windows_has_tzdata(),
reason="Timezone database is not installed on Windows")
def test_strftime():
times = ["2018-03-10 09:00", "2038-01-31 12:23", None]
timezones = ["CET", "UTC", "Europe/Ljubljana"]
formats = ["%a", "%A", "%w", "%d", "%b", "%B", "%m", "%y", "%Y", "%H",
"%I", "%p", "%M", "%z", "%Z", "%j", "%U", "%W", "%c", "%x",
"%X", "%%", "%G", "%V", "%u"]
for timezone in timezones:
ts = pd.to_datetime(times).tz_localize(timezone)
for unit in ["s", "ms", "us", "ns"]:
tsa = pa.array(ts, type=pa.timestamp(unit, timezone))
for fmt in formats:
options = pc.StrftimeOptions(fmt)
result = pc.strftime(tsa, options=options)
expected = pa.array(ts.strftime(fmt))
> assert result.equals(expected)
E assert False
E + where False = <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600>(<pyarrow.lib.StringArray object at 0x0000023767338830>\n[\n "Sat Mar 10 09:00:00 2018",\n "Sun Jan 31 12:23:00 2038",\n null\n])
E + where <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600> = <pyarrow.lib.StringArray object at 0x0000023767338600>\n[\n "03/10/18 09:00:00",\n "01/31/38 12:23:00",\n null\n].equals
pyarrow\tests\test_compute.py:1872: AssertionError

So it seems that we create a string like "Sat Mar 10 09:00:00 2018", but the python version we compare with gives "03/10/18 09:00:00". According to docs for %c, the former (our result) is actually correct.
But since we are checking matching results in Python in this test, just skipping the ones where those don't match.

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels May 31, 2023
@jorisvandenbossche

jorisvandenbossche commented May 31, 2023

Copy link
Copy Markdown
MemberAuthor

I updated the PR to actually check if the tzdata database was present, and only skip the tests if not. Because we do actually already download the tzdb on Appveyor (not on the nightly wheel builds, so therefore the tests were failing there).
While Python doesn't yet allow you to configure the path (and use the data from the tzdata package), as mentioned above, if you manually download it to the correct location, the C++ code will already discover it (as was done for R).

@jorisvandenbossche

jorisvandenbossche commented Jun 6, 2023

Copy link
Copy Markdown
MemberAuthor

@github-actions crossbow submit -g wheel

@github-actions

This comment was marked as outdated.

Sign in to view
@jorisvandenbossche

jorisvandenbossche commented Jun 6, 2023

Copy link
Copy Markdown
MemberAuthor

@github-actions crossbow submit -g wheel

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 6, 2023
@github-actions

github-actionsBot commented Jun 6, 2023

Copy link
Copy Markdown

Revision: 9720e36

Submitted crossbow builds: ursacomputing/crossbow @ actions-5e510fd809

TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions

@jorisvandenbossche
jorisvandenbossche merged commit c78ef0f into apache:mainJun 6, 2023
@jorisvandenbossche
jorisvandenbossche deleted the GH-35040-fix-ci branch June 6, 2023 15:24
@ursabot

ursabot commented Jun 7, 2023

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 4ec231b and contender = c78ef0f. c78ef0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.77% ⬆️0.06%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Failed ⬇️0.27% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] c78ef0f1 ec2-t3-xlarge-us-east-2
[Finished] c78ef0f1 test-mac-arm
[Finished] c78ef0f1 ursa-i9-9960x
[Failed] c78ef0f1 ursa-thinkcentre-m75q
[Failed] 4ec231b2 ec2-t3-xlarge-us-east-2
[Failed] 4ec231b2 test-mac-arm
[Finished] 4ec231b2 ursa-i9-9960x
[Failed] 4ec231b2 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@mattijnmattijn mentioned this pull request Jun 7, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rokrokrok approved these changes
@raulcdraulcdraulcd approved these changes
@wgtmacwgtmacwgtmac approved these changes
@AlenkaFAlenkaFAwaiting requested review from AlenkaFAlenkaF is a code owner
@danepitkindanepitkinAwaiting requested review from danepitkin

Assignees

No one assigned

Labels

awaiting change reviewAwaiting change reviewComponent: Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@jorisvandenbossche@pitrou@danepitkin@wjones127@ursabot@rok@raulcd@wgtmac
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database by jorisvandenbossche · Pull Request #35735 · apache/arrow · GitHub
Skip to content
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database - #35735
Merged
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Jun 6, 2023
Merged
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database#35735
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Conversation
@jorisvandenbossche
@jorisvandenbosschejorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Rationale for this change
Fix up of #35395, skipping one of the tests added in that PR on Windows, because the test requires access to a tz database.
@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@jorisvandenbosschejorisvandenbossche
mentioned this pull request
May 24, 2023
 Merged

@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

Revision: 64c0d83
Submitted crossbow builds: ursacomputing/crossbow @ actions-fe7da097ff
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@pitrou
pitrou
requested a review
from rokMay 24, 2023 10:33
@pitrou
pitrou
commented
May 24, 2023

Copy link


Copy Markdown

Member
@rok does this look right? We are able to somehow use a timezone database on Windows, IIRC?
@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Yeah, the better fix would actually be to detect if the database is available, and only skip if not found on windows. Because this patch skips the tests altogether, even when it can actually work (eg on appveyor the tests pass because we download the tzdata during appveyor setup). But just to note: we currently do this plain skip for other tests requiring the database as well, so that is an existing issue with our test setup.
I assume checking if the path %USERPROFILE%\Downloads\tzdata exists might be a sufficient check at the moment (since that's the place where tz.cpp will look, and we currently don't yet allow customizing that in pyarrow).
@danepitkin
danepitkin
commented
May 24, 2023

Copy link


Copy Markdown

Member
Thanks for fixing! There are other tests in this file that also use pytest.importorskip("pytz"), should they be updated as well? It's not clear to me why this test case behaves differently.
rok
rok
reviewed
May 24, 2023
View reviewed changes
@rokrok
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

For completeness - I believe db download for windows was added by @wjones127 in #12536.
Comment threadpython/pyarrow/tests/test_scalars.py
Outdated

@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
May 24, 2023
@wjones127
wjones127
commented
May 25, 2023

Copy link


Copy Markdown

Member
For completeness - I believe db download for windows was added by @wjones127 in #12536.
That was just for C++ and R. I don't think we figured out how to do it for Python, since we need the text form of the IANA database while Python's timezone packages provide the binary form.
raulcd
raulcd
approved these changes
May 27, 2023
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

We already have 3 tests being skipped on Windows for the same reason.
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L1851
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042

I am +1 to merge
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
May 27, 2023
rok
rok
approved these changes
May 27, 2023
View reviewed changes
jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023
View reviewed changes
Comment threadpython/pyarrow/tests/test_compute.py

Comment on lines

+1862
 to 
+1864

 if sys.platform != "win32":
 # Locale-dependent formats don't match on Windows
 formats.extend(["%c", "%x", "%X"])
@jorisvandenbosschejorisvandenbosscheMay 31, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

The error on Appveyor we got was:
 @pytest.mark.skipif(sys.platform == "win32" and not util.windows_has_tzdata(),
reason="Timezone database is not installed on Windows")
def test_strftime():
times = ["2018-03-10 09:00", "2038-01-31 12:23", None]
timezones = ["CET", "UTC", "Europe/Ljubljana"]
formats = ["%a", "%A", "%w", "%d", "%b", "%B", "%m", "%y", "%Y", "%H",
"%I", "%p", "%M", "%z", "%Z", "%j", "%U", "%W", "%c", "%x",
"%X", "%%", "%G", "%V", "%u"]
for timezone in timezones:
ts = pd.to_datetime(times).tz_localize(timezone)
for unit in ["s", "ms", "us", "ns"]:
tsa = pa.array(ts, type=pa.timestamp(unit, timezone))
for fmt in formats:
options = pc.StrftimeOptions(fmt)
result = pc.strftime(tsa, options=options)
expected = pa.array(ts.strftime(fmt))
> assert result.equals(expected)
E assert False
E + where False = <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600>(<pyarrow.lib.StringArray object at 0x0000023767338830>\n[\n "Sat Mar 10 09:00:00 2018",\n "Sun Jan 31 12:23:00 2038",\n null\n])
E + where <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600> = <pyarrow.lib.StringArray object at 0x0000023767338600>\n[\n "03/10/18 09:00:00",\n "01/31/38 12:23:00",\n null\n].equals
pyarrow\tests\test_compute.py:1872: AssertionError

So it seems that we create a string like "Sat Mar 10 09:00:00 2018", but the python version we compare with gives "03/10/18 09:00:00". According to docs for %c, the former (our result) is actually correct.

But since we are checking matching results in Python in this test, just skipping the ones where those don't match.
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
May 31, 2023
@jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023

Copy link


Copy Markdown

MemberAuthor
I updated the PR to actually check if the tzdata database was present, and only skip the tests if not. Because we do actually already download the tzdb on Appveyor (not on the nightly wheel builds, so therefore the tests were failing there).

While Python doesn't yet allow you to configure the path (and use the data from the tzdata package), as mentioned above, if you manually download it to the correct location, the C++ code will already discover it (as was done for R).
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actions

This comment was marked as outdated.

Sign in to view
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actionsgithub-actionsBot
added

awaiting change review
Awaiting change review
and removed

awaiting changes
Awaiting changes
labels
Jun 6, 2023
@github-actions
github-actionsBot
commented
Jun 6, 2023

Copy link


Copy Markdown

Revision: 9720e36
Submitted crossbow builds: ursacomputing/crossbow @ actions-5e510fd809
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@jorisvandenbossche
jorisvandenbossche
merged commit c78ef0f
into
apache:mainJun 6, 2023
@jorisvandenbossche
jorisvandenbossche
deleted the

GH-35040-fix-ci

branch
June 6, 2023 15:24
@ursabot
ursabot
commented
Jun 7, 2023

Copy link


Copy Markdown

Benchmark runs are scheduled for baseline = 4ec231b and contender = c78ef0f. c78ef0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.

Conbench compare runs links:

[Failed] ec2-t3-xlarge-us-east-2

[Failed ⬇️0.77% ⬆️0.06%] test-mac-arm

[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x

[Failed ⬇️0.27% ⬆️0.0%] ursa-thinkcentre-m75q

Buildkite builds:

[Finished] c78ef0f1 ec2-t3-xlarge-us-east-2

[Finished] c78ef0f1 test-mac-arm

[Finished] c78ef0f1 ursa-i9-9960x

[Failed] c78ef0f1 ursa-thinkcentre-m75q

[Failed] 4ec231b2 ec2-t3-xlarge-us-east-2

[Failed] 4ec231b2 test-mac-arm

[Finished] 4ec231b2 ursa-i9-9960x

[Failed] 4ec231b2 ursa-thinkcentre-m75q

Supported benchmarks:

ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True

test-mac-arm: Supported benchmark langs: C++, Python, R

ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript

ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
@mattijnmattijn
mentioned this pull request
Jun 7, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rokrokrok approved these changes
@raulcdraulcdraulcd approved these changes
@wgtmacwgtmacwgtmac approved these changes
@AlenkaFAlenkaFAwaiting requested review from AlenkaFAlenkaF is a code owner
@danepitkindanepitkinAwaiting requested review from danepitkin

Assignees


No one assigned


Labels

awaiting change reviewAwaiting change reviewComponent: Python

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@jorisvandenbossche@pitrou@danepitkin@wjones127@ursabot@rok@raulcd@wgtmac

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database by jorisvandenbossche · Pull Request #35735 · apache/arrow · GitHub
Skip to content
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database - #35735
Merged
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Jun 6, 2023
Merged
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database#35735
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Conversation
@jorisvandenbossche
@jorisvandenbosschejorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Rationale for this change
Fix up of #35395, skipping one of the tests added in that PR on Windows, because the test requires access to a tz database.
@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@jorisvandenbosschejorisvandenbossche
mentioned this pull request
May 24, 2023
 Merged

@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

Revision: 64c0d83
Submitted crossbow builds: ursacomputing/crossbow @ actions-fe7da097ff
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@pitrou
pitrou
requested a review
from rokMay 24, 2023 10:33
@pitrou
pitrou
commented
May 24, 2023

Copy link


Copy Markdown

Member
@rok does this look right? We are able to somehow use a timezone database on Windows, IIRC?
@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Yeah, the better fix would actually be to detect if the database is available, and only skip if not found on windows. Because this patch skips the tests altogether, even when it can actually work (eg on appveyor the tests pass because we download the tzdata during appveyor setup). But just to note: we currently do this plain skip for other tests requiring the database as well, so that is an existing issue with our test setup.
I assume checking if the path %USERPROFILE%\Downloads\tzdata exists might be a sufficient check at the moment (since that's the place where tz.cpp will look, and we currently don't yet allow customizing that in pyarrow).
@danepitkin
danepitkin
commented
May 24, 2023

Copy link


Copy Markdown

Member
Thanks for fixing! There are other tests in this file that also use pytest.importorskip("pytz"), should they be updated as well? It's not clear to me why this test case behaves differently.
rok
rok
reviewed
May 24, 2023
View reviewed changes
@rokrok
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

For completeness - I believe db download for windows was added by @wjones127 in #12536.
Comment threadpython/pyarrow/tests/test_scalars.py
Outdated

@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
May 24, 2023
@wjones127
wjones127
commented
May 25, 2023

Copy link


Copy Markdown

Member
For completeness - I believe db download for windows was added by @wjones127 in #12536.
That was just for C++ and R. I don't think we figured out how to do it for Python, since we need the text form of the IANA database while Python's timezone packages provide the binary form.
raulcd
raulcd
approved these changes
May 27, 2023
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

We already have 3 tests being skipped on Windows for the same reason.
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L1851
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042

I am +1 to merge
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
May 27, 2023
rok
rok
approved these changes
May 27, 2023
View reviewed changes
jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023
View reviewed changes
Comment threadpython/pyarrow/tests/test_compute.py

Comment on lines

+1862
 to 
+1864

 if sys.platform != "win32":
 # Locale-dependent formats don't match on Windows
 formats.extend(["%c", "%x", "%X"])
@jorisvandenbosschejorisvandenbosscheMay 31, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

The error on Appveyor we got was:
 @pytest.mark.skipif(sys.platform == "win32" and not util.windows_has_tzdata(),
reason="Timezone database is not installed on Windows")
def test_strftime():
times = ["2018-03-10 09:00", "2038-01-31 12:23", None]
timezones = ["CET", "UTC", "Europe/Ljubljana"]
formats = ["%a", "%A", "%w", "%d", "%b", "%B", "%m", "%y", "%Y", "%H",
"%I", "%p", "%M", "%z", "%Z", "%j", "%U", "%W", "%c", "%x",
"%X", "%%", "%G", "%V", "%u"]
for timezone in timezones:
ts = pd.to_datetime(times).tz_localize(timezone)
for unit in ["s", "ms", "us", "ns"]:
tsa = pa.array(ts, type=pa.timestamp(unit, timezone))
for fmt in formats:
options = pc.StrftimeOptions(fmt)
result = pc.strftime(tsa, options=options)
expected = pa.array(ts.strftime(fmt))
> assert result.equals(expected)
E assert False
E + where False = <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600>(<pyarrow.lib.StringArray object at 0x0000023767338830>\n[\n "Sat Mar 10 09:00:00 2018",\n "Sun Jan 31 12:23:00 2038",\n null\n])
E + where <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600> = <pyarrow.lib.StringArray object at 0x0000023767338600>\n[\n "03/10/18 09:00:00",\n "01/31/38 12:23:00",\n null\n].equals
pyarrow\tests\test_compute.py:1872: AssertionError

So it seems that we create a string like "Sat Mar 10 09:00:00 2018", but the python version we compare with gives "03/10/18 09:00:00". According to docs for %c, the former (our result) is actually correct.

But since we are checking matching results in Python in this test, just skipping the ones where those don't match.
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
May 31, 2023
@jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023

Copy link


Copy Markdown

MemberAuthor
I updated the PR to actually check if the tzdata database was present, and only skip the tests if not. Because we do actually already download the tzdb on Appveyor (not on the nightly wheel builds, so therefore the tests were failing there).

While Python doesn't yet allow you to configure the path (and use the data from the tzdata package), as mentioned above, if you manually download it to the correct location, the C++ code will already discover it (as was done for R).
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actions

This comment was marked as outdated.

Sign in to view
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actionsgithub-actionsBot
added

awaiting change review
Awaiting change review
and removed

awaiting changes
Awaiting changes
labels
Jun 6, 2023
@github-actions
github-actionsBot
commented
Jun 6, 2023

Copy link


Copy Markdown

Revision: 9720e36
Submitted crossbow builds: ursacomputing/crossbow @ actions-5e510fd809
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@jorisvandenbossche
jorisvandenbossche
merged commit c78ef0f
into
apache:mainJun 6, 2023
@jorisvandenbossche
jorisvandenbossche
deleted the

GH-35040-fix-ci

branch
June 6, 2023 15:24
@ursabot
ursabot
commented
Jun 7, 2023

Copy link


Copy Markdown

Benchmark runs are scheduled for baseline = 4ec231b and contender = c78ef0f. c78ef0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.

Conbench compare runs links:

[Failed] ec2-t3-xlarge-us-east-2

[Failed ⬇️0.77% ⬆️0.06%] test-mac-arm

[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x

[Failed ⬇️0.27% ⬆️0.0%] ursa-thinkcentre-m75q

Buildkite builds:

[Finished] c78ef0f1 ec2-t3-xlarge-us-east-2

[Finished] c78ef0f1 test-mac-arm

[Finished] c78ef0f1 ursa-i9-9960x

[Failed] c78ef0f1 ursa-thinkcentre-m75q

[Failed] 4ec231b2 ec2-t3-xlarge-us-east-2

[Failed] 4ec231b2 test-mac-arm

[Finished] 4ec231b2 ursa-i9-9960x

[Failed] 4ec231b2 ursa-thinkcentre-m75q

Supported benchmarks:

ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True

test-mac-arm: Supported benchmark langs: C++, Python, R

ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript

ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
@mattijnmattijn
mentioned this pull request
Jun 7, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rokrokrok approved these changes
@raulcdraulcdraulcd approved these changes
@wgtmacwgtmacwgtmac approved these changes
@AlenkaFAlenkaFAwaiting requested review from AlenkaFAlenkaF is a code owner
@danepitkindanepitkinAwaiting requested review from danepitkin

Assignees


No one assigned


Labels

awaiting change reviewAwaiting change reviewComponent: Python

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@jorisvandenbossche@pitrou@danepitkin@wjones127@ursabot@rok@raulcd@wgtmac

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database by jorisvandenbossche · Pull Request #35735 · apache/arrow · GitHub
Skip to content
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database - #35735
Merged
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Jun 6, 2023
Merged
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database#35735
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Conversation
@jorisvandenbossche
@jorisvandenbosschejorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Rationale for this change
Fix up of #35395, skipping one of the tests added in that PR on Windows, because the test requires access to a tz database.
@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@jorisvandenbosschejorisvandenbossche
mentioned this pull request
May 24, 2023
 Merged

@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

Revision: 64c0d83
Submitted crossbow builds: ursacomputing/crossbow @ actions-fe7da097ff
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@pitrou
pitrou
requested a review
from rokMay 24, 2023 10:33
@pitrou
pitrou
commented
May 24, 2023

Copy link


Copy Markdown

Member
@rok does this look right? We are able to somehow use a timezone database on Windows, IIRC?
@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Yeah, the better fix would actually be to detect if the database is available, and only skip if not found on windows. Because this patch skips the tests altogether, even when it can actually work (eg on appveyor the tests pass because we download the tzdata during appveyor setup). But just to note: we currently do this plain skip for other tests requiring the database as well, so that is an existing issue with our test setup.
I assume checking if the path %USERPROFILE%\Downloads\tzdata exists might be a sufficient check at the moment (since that's the place where tz.cpp will look, and we currently don't yet allow customizing that in pyarrow).
@danepitkin
danepitkin
commented
May 24, 2023

Copy link


Copy Markdown

Member
Thanks for fixing! There are other tests in this file that also use pytest.importorskip("pytz"), should they be updated as well? It's not clear to me why this test case behaves differently.
rok
rok
reviewed
May 24, 2023
View reviewed changes
@rokrok
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

For completeness - I believe db download for windows was added by @wjones127 in #12536.
Comment threadpython/pyarrow/tests/test_scalars.py
Outdated

@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
May 24, 2023
@wjones127
wjones127
commented
May 25, 2023

Copy link


Copy Markdown

Member
For completeness - I believe db download for windows was added by @wjones127 in #12536.
That was just for C++ and R. I don't think we figured out how to do it for Python, since we need the text form of the IANA database while Python's timezone packages provide the binary form.
raulcd
raulcd
approved these changes
May 27, 2023
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

We already have 3 tests being skipped on Windows for the same reason.
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L1851
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042

I am +1 to merge
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
May 27, 2023
rok
rok
approved these changes
May 27, 2023
View reviewed changes
jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023
View reviewed changes
Comment threadpython/pyarrow/tests/test_compute.py

Comment on lines

+1862
 to 
+1864

 if sys.platform != "win32":
 # Locale-dependent formats don't match on Windows
 formats.extend(["%c", "%x", "%X"])
@jorisvandenbosschejorisvandenbosscheMay 31, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

The error on Appveyor we got was:
 @pytest.mark.skipif(sys.platform == "win32" and not util.windows_has_tzdata(),
reason="Timezone database is not installed on Windows")
def test_strftime():
times = ["2018-03-10 09:00", "2038-01-31 12:23", None]
timezones = ["CET", "UTC", "Europe/Ljubljana"]
formats = ["%a", "%A", "%w", "%d", "%b", "%B", "%m", "%y", "%Y", "%H",
"%I", "%p", "%M", "%z", "%Z", "%j", "%U", "%W", "%c", "%x",
"%X", "%%", "%G", "%V", "%u"]
for timezone in timezones:
ts = pd.to_datetime(times).tz_localize(timezone)
for unit in ["s", "ms", "us", "ns"]:
tsa = pa.array(ts, type=pa.timestamp(unit, timezone))
for fmt in formats:
options = pc.StrftimeOptions(fmt)
result = pc.strftime(tsa, options=options)
expected = pa.array(ts.strftime(fmt))
> assert result.equals(expected)
E assert False
E + where False = <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600>(<pyarrow.lib.StringArray object at 0x0000023767338830>\n[\n "Sat Mar 10 09:00:00 2018",\n "Sun Jan 31 12:23:00 2038",\n null\n])
E + where <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600> = <pyarrow.lib.StringArray object at 0x0000023767338600>\n[\n "03/10/18 09:00:00",\n "01/31/38 12:23:00",\n null\n].equals
pyarrow\tests\test_compute.py:1872: AssertionError

So it seems that we create a string like "Sat Mar 10 09:00:00 2018", but the python version we compare with gives "03/10/18 09:00:00". According to docs for %c, the former (our result) is actually correct.

But since we are checking matching results in Python in this test, just skipping the ones where those don't match.
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
May 31, 2023
@jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023

Copy link


Copy Markdown

MemberAuthor
I updated the PR to actually check if the tzdata database was present, and only skip the tests if not. Because we do actually already download the tzdb on Appveyor (not on the nightly wheel builds, so therefore the tests were failing there).

While Python doesn't yet allow you to configure the path (and use the data from the tzdata package), as mentioned above, if you manually download it to the correct location, the C++ code will already discover it (as was done for R).
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actions

This comment was marked as outdated.

Sign in to view
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actionsgithub-actionsBot
added

awaiting change review
Awaiting change review
and removed

awaiting changes
Awaiting changes
labels
Jun 6, 2023
@github-actions
github-actionsBot
commented
Jun 6, 2023

Copy link


Copy Markdown

Revision: 9720e36
Submitted crossbow builds: ursacomputing/crossbow @ actions-5e510fd809
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@jorisvandenbossche
jorisvandenbossche
merged commit c78ef0f
into
apache:mainJun 6, 2023
@jorisvandenbossche
jorisvandenbossche
deleted the

GH-35040-fix-ci

branch
June 6, 2023 15:24
@ursabot
ursabot
commented
Jun 7, 2023

Copy link


Copy Markdown

Benchmark runs are scheduled for baseline = 4ec231b and contender = c78ef0f. c78ef0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.

Conbench compare runs links:

[Failed] ec2-t3-xlarge-us-east-2

[Failed ⬇️0.77% ⬆️0.06%] test-mac-arm

[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x

[Failed ⬇️0.27% ⬆️0.0%] ursa-thinkcentre-m75q

Buildkite builds:

[Finished] c78ef0f1 ec2-t3-xlarge-us-east-2

[Finished] c78ef0f1 test-mac-arm

[Finished] c78ef0f1 ursa-i9-9960x

[Failed] c78ef0f1 ursa-thinkcentre-m75q

[Failed] 4ec231b2 ec2-t3-xlarge-us-east-2

[Failed] 4ec231b2 test-mac-arm

[Finished] 4ec231b2 ursa-i9-9960x

[Failed] 4ec231b2 ursa-thinkcentre-m75q

Supported benchmarks:

ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True

test-mac-arm: Supported benchmark langs: C++, Python, R

ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript

ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
@mattijnmattijn
mentioned this pull request
Jun 7, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rokrokrok approved these changes
@raulcdraulcdraulcd approved these changes
@wgtmacwgtmacwgtmac approved these changes
@AlenkaFAlenkaFAwaiting requested review from AlenkaFAlenkaF is a code owner
@danepitkindanepitkinAwaiting requested review from danepitkin

Assignees


No one assigned


Labels

awaiting change reviewAwaiting change reviewComponent: Python

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@jorisvandenbossche@pitrou@danepitkin@wjones127@ursabot@rok@raulcd@wgtmac

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database by jorisvandenbossche · Pull Request #35735 · apache/arrow · GitHub
Skip to content
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database - #35735
Merged
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Jun 6, 2023
Merged
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database#35735
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Conversation
@jorisvandenbossche
@jorisvandenbosschejorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Rationale for this change
Fix up of #35395, skipping one of the tests added in that PR on Windows, because the test requires access to a tz database.
@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@jorisvandenbosschejorisvandenbossche
mentioned this pull request
May 24, 2023
 Merged

@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

Revision: 64c0d83
Submitted crossbow builds: ursacomputing/crossbow @ actions-fe7da097ff
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@pitrou
pitrou
requested a review
from rokMay 24, 2023 10:33
@pitrou
pitrou
commented
May 24, 2023

Copy link


Copy Markdown

Member
@rok does this look right? We are able to somehow use a timezone database on Windows, IIRC?
@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Yeah, the better fix would actually be to detect if the database is available, and only skip if not found on windows. Because this patch skips the tests altogether, even when it can actually work (eg on appveyor the tests pass because we download the tzdata during appveyor setup). But just to note: we currently do this plain skip for other tests requiring the database as well, so that is an existing issue with our test setup.
I assume checking if the path %USERPROFILE%\Downloads\tzdata exists might be a sufficient check at the moment (since that's the place where tz.cpp will look, and we currently don't yet allow customizing that in pyarrow).
@danepitkin
danepitkin
commented
May 24, 2023

Copy link


Copy Markdown

Member
Thanks for fixing! There are other tests in this file that also use pytest.importorskip("pytz"), should they be updated as well? It's not clear to me why this test case behaves differently.
rok
rok
reviewed
May 24, 2023
View reviewed changes
@rokrok
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

For completeness - I believe db download for windows was added by @wjones127 in #12536.
Comment threadpython/pyarrow/tests/test_scalars.py
Outdated

@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
May 24, 2023
@wjones127
wjones127
commented
May 25, 2023

Copy link


Copy Markdown

Member
For completeness - I believe db download for windows was added by @wjones127 in #12536.
That was just for C++ and R. I don't think we figured out how to do it for Python, since we need the text form of the IANA database while Python's timezone packages provide the binary form.
raulcd
raulcd
approved these changes
May 27, 2023
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

We already have 3 tests being skipped on Windows for the same reason.
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L1851
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042

I am +1 to merge
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
May 27, 2023
rok
rok
approved these changes
May 27, 2023
View reviewed changes
jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023
View reviewed changes
Comment threadpython/pyarrow/tests/test_compute.py

Comment on lines

+1862
 to 
+1864

 if sys.platform != "win32":
 # Locale-dependent formats don't match on Windows
 formats.extend(["%c", "%x", "%X"])
@jorisvandenbosschejorisvandenbosscheMay 31, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

The error on Appveyor we got was:
 @pytest.mark.skipif(sys.platform == "win32" and not util.windows_has_tzdata(),
reason="Timezone database is not installed on Windows")
def test_strftime():
times = ["2018-03-10 09:00", "2038-01-31 12:23", None]
timezones = ["CET", "UTC", "Europe/Ljubljana"]
formats = ["%a", "%A", "%w", "%d", "%b", "%B", "%m", "%y", "%Y", "%H",
"%I", "%p", "%M", "%z", "%Z", "%j", "%U", "%W", "%c", "%x",
"%X", "%%", "%G", "%V", "%u"]
for timezone in timezones:
ts = pd.to_datetime(times).tz_localize(timezone)
for unit in ["s", "ms", "us", "ns"]:
tsa = pa.array(ts, type=pa.timestamp(unit, timezone))
for fmt in formats:
options = pc.StrftimeOptions(fmt)
result = pc.strftime(tsa, options=options)
expected = pa.array(ts.strftime(fmt))
> assert result.equals(expected)
E assert False
E + where False = <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600>(<pyarrow.lib.StringArray object at 0x0000023767338830>\n[\n "Sat Mar 10 09:00:00 2018",\n "Sun Jan 31 12:23:00 2038",\n null\n])
E + where <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600> = <pyarrow.lib.StringArray object at 0x0000023767338600>\n[\n "03/10/18 09:00:00",\n "01/31/38 12:23:00",\n null\n].equals
pyarrow\tests\test_compute.py:1872: AssertionError

So it seems that we create a string like "Sat Mar 10 09:00:00 2018", but the python version we compare with gives "03/10/18 09:00:00". According to docs for %c, the former (our result) is actually correct.

But since we are checking matching results in Python in this test, just skipping the ones where those don't match.
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
May 31, 2023
@jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023

Copy link


Copy Markdown

MemberAuthor
I updated the PR to actually check if the tzdata database was present, and only skip the tests if not. Because we do actually already download the tzdb on Appveyor (not on the nightly wheel builds, so therefore the tests were failing there).

While Python doesn't yet allow you to configure the path (and use the data from the tzdata package), as mentioned above, if you manually download it to the correct location, the C++ code will already discover it (as was done for R).
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actions

This comment was marked as outdated.

Sign in to view
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actionsgithub-actionsBot
added

awaiting change review
Awaiting change review
and removed

awaiting changes
Awaiting changes
labels
Jun 6, 2023
@github-actions
github-actionsBot
commented
Jun 6, 2023

Copy link


Copy Markdown

Revision: 9720e36
Submitted crossbow builds: ursacomputing/crossbow @ actions-5e510fd809
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@jorisvandenbossche
jorisvandenbossche
merged commit c78ef0f
into
apache:mainJun 6, 2023
@jorisvandenbossche
jorisvandenbossche
deleted the

GH-35040-fix-ci

branch
June 6, 2023 15:24
@ursabot
ursabot
commented
Jun 7, 2023

Copy link


Copy Markdown

Benchmark runs are scheduled for baseline = 4ec231b and contender = c78ef0f. c78ef0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.

Conbench compare runs links:

[Failed] ec2-t3-xlarge-us-east-2

[Failed ⬇️0.77% ⬆️0.06%] test-mac-arm

[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x

[Failed ⬇️0.27% ⬆️0.0%] ursa-thinkcentre-m75q

Buildkite builds:

[Finished] c78ef0f1 ec2-t3-xlarge-us-east-2

[Finished] c78ef0f1 test-mac-arm

[Finished] c78ef0f1 ursa-i9-9960x

[Failed] c78ef0f1 ursa-thinkcentre-m75q

[Failed] 4ec231b2 ec2-t3-xlarge-us-east-2

[Failed] 4ec231b2 test-mac-arm

[Finished] 4ec231b2 ursa-i9-9960x

[Failed] 4ec231b2 ursa-thinkcentre-m75q

Supported benchmarks:

ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True

test-mac-arm: Supported benchmark langs: C++, Python, R

ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript

ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
@mattijnmattijn
mentioned this pull request
Jun 7, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rokrokrok approved these changes
@raulcdraulcdraulcd approved these changes
@wgtmacwgtmacwgtmac approved these changes
@AlenkaFAlenkaFAwaiting requested review from AlenkaFAlenkaF is a code owner
@danepitkindanepitkinAwaiting requested review from danepitkin

Assignees


No one assigned


Labels

awaiting change reviewAwaiting change reviewComponent: Python

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@jorisvandenbossche@pitrou@danepitkin@wjones127@ursabot@rok@raulcd@wgtmac

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database by jorisvandenbossche · Pull Request #35735 · apache/arrow · GitHub
Skip to content
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database - #35735
Merged
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Jun 6, 2023
Merged
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database#35735
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Conversation
@jorisvandenbossche
@jorisvandenbosschejorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Rationale for this change
Fix up of #35395, skipping one of the tests added in that PR on Windows, because the test requires access to a tz database.
@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@jorisvandenbosschejorisvandenbossche
mentioned this pull request
May 24, 2023
 Merged

@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

Revision: 64c0d83
Submitted crossbow builds: ursacomputing/crossbow @ actions-fe7da097ff
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@pitrou
pitrou
requested a review
from rokMay 24, 2023 10:33
@pitrou
pitrou
commented
May 24, 2023

Copy link


Copy Markdown

Member
@rok does this look right? We are able to somehow use a timezone database on Windows, IIRC?
@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Yeah, the better fix would actually be to detect if the database is available, and only skip if not found on windows. Because this patch skips the tests altogether, even when it can actually work (eg on appveyor the tests pass because we download the tzdata during appveyor setup). But just to note: we currently do this plain skip for other tests requiring the database as well, so that is an existing issue with our test setup.
I assume checking if the path %USERPROFILE%\Downloads\tzdata exists might be a sufficient check at the moment (since that's the place where tz.cpp will look, and we currently don't yet allow customizing that in pyarrow).
@danepitkin
danepitkin
commented
May 24, 2023

Copy link


Copy Markdown

Member
Thanks for fixing! There are other tests in this file that also use pytest.importorskip("pytz"), should they be updated as well? It's not clear to me why this test case behaves differently.
rok
rok
reviewed
May 24, 2023
View reviewed changes
@rokrok
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

For completeness - I believe db download for windows was added by @wjones127 in #12536.
Comment threadpython/pyarrow/tests/test_scalars.py
Outdated

@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
May 24, 2023
@wjones127
wjones127
commented
May 25, 2023

Copy link


Copy Markdown

Member
For completeness - I believe db download for windows was added by @wjones127 in #12536.
That was just for C++ and R. I don't think we figured out how to do it for Python, since we need the text form of the IANA database while Python's timezone packages provide the binary form.
raulcd
raulcd
approved these changes
May 27, 2023
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

We already have 3 tests being skipped on Windows for the same reason.
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L1851
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042

I am +1 to merge
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
May 27, 2023
rok
rok
approved these changes
May 27, 2023
View reviewed changes
jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023
View reviewed changes
Comment threadpython/pyarrow/tests/test_compute.py

Comment on lines

+1862
 to 
+1864

 if sys.platform != "win32":
 # Locale-dependent formats don't match on Windows
 formats.extend(["%c", "%x", "%X"])
@jorisvandenbosschejorisvandenbosscheMay 31, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

The error on Appveyor we got was:
 @pytest.mark.skipif(sys.platform == "win32" and not util.windows_has_tzdata(),
reason="Timezone database is not installed on Windows")
def test_strftime():
times = ["2018-03-10 09:00", "2038-01-31 12:23", None]
timezones = ["CET", "UTC", "Europe/Ljubljana"]
formats = ["%a", "%A", "%w", "%d", "%b", "%B", "%m", "%y", "%Y", "%H",
"%I", "%p", "%M", "%z", "%Z", "%j", "%U", "%W", "%c", "%x",
"%X", "%%", "%G", "%V", "%u"]
for timezone in timezones:
ts = pd.to_datetime(times).tz_localize(timezone)
for unit in ["s", "ms", "us", "ns"]:
tsa = pa.array(ts, type=pa.timestamp(unit, timezone))
for fmt in formats:
options = pc.StrftimeOptions(fmt)
result = pc.strftime(tsa, options=options)
expected = pa.array(ts.strftime(fmt))
> assert result.equals(expected)
E assert False
E + where False = <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600>(<pyarrow.lib.StringArray object at 0x0000023767338830>\n[\n "Sat Mar 10 09:00:00 2018",\n "Sun Jan 31 12:23:00 2038",\n null\n])
E + where <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600> = <pyarrow.lib.StringArray object at 0x0000023767338600>\n[\n "03/10/18 09:00:00",\n "01/31/38 12:23:00",\n null\n].equals
pyarrow\tests\test_compute.py:1872: AssertionError

So it seems that we create a string like "Sat Mar 10 09:00:00 2018", but the python version we compare with gives "03/10/18 09:00:00". According to docs for %c, the former (our result) is actually correct.

But since we are checking matching results in Python in this test, just skipping the ones where those don't match.
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
May 31, 2023
@jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023

Copy link


Copy Markdown

MemberAuthor
I updated the PR to actually check if the tzdata database was present, and only skip the tests if not. Because we do actually already download the tzdb on Appveyor (not on the nightly wheel builds, so therefore the tests were failing there).

While Python doesn't yet allow you to configure the path (and use the data from the tzdata package), as mentioned above, if you manually download it to the correct location, the C++ code will already discover it (as was done for R).
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actions

This comment was marked as outdated.

Sign in to view
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actionsgithub-actionsBot
added

awaiting change review
Awaiting change review
and removed

awaiting changes
Awaiting changes
labels
Jun 6, 2023
@github-actions
github-actionsBot
commented
Jun 6, 2023

Copy link


Copy Markdown

Revision: 9720e36
Submitted crossbow builds: ursacomputing/crossbow @ actions-5e510fd809
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@jorisvandenbossche
jorisvandenbossche
merged commit c78ef0f
into
apache:mainJun 6, 2023
@jorisvandenbossche
jorisvandenbossche
deleted the

GH-35040-fix-ci

branch
June 6, 2023 15:24
@ursabot
ursabot
commented
Jun 7, 2023

Copy link


Copy Markdown

Benchmark runs are scheduled for baseline = 4ec231b and contender = c78ef0f. c78ef0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.

Conbench compare runs links:

[Failed] ec2-t3-xlarge-us-east-2

[Failed ⬇️0.77% ⬆️0.06%] test-mac-arm

[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x

[Failed ⬇️0.27% ⬆️0.0%] ursa-thinkcentre-m75q

Buildkite builds:

[Finished] c78ef0f1 ec2-t3-xlarge-us-east-2

[Finished] c78ef0f1 test-mac-arm

[Finished] c78ef0f1 ursa-i9-9960x

[Failed] c78ef0f1 ursa-thinkcentre-m75q

[Failed] 4ec231b2 ec2-t3-xlarge-us-east-2

[Failed] 4ec231b2 test-mac-arm

[Finished] 4ec231b2 ursa-i9-9960x

[Failed] 4ec231b2 ursa-thinkcentre-m75q

Supported benchmarks:

ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True

test-mac-arm: Supported benchmark langs: C++, Python, R

ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript

ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
@mattijnmattijn
mentioned this pull request
Jun 7, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rokrokrok approved these changes
@raulcdraulcdraulcd approved these changes
@wgtmacwgtmacwgtmac approved these changes
@AlenkaFAlenkaFAwaiting requested review from AlenkaFAlenkaF is a code owner
@danepitkindanepitkinAwaiting requested review from danepitkin

Assignees


No one assigned


Labels

awaiting change reviewAwaiting change reviewComponent: Python

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@jorisvandenbossche@pitrou@danepitkin@wjones127@ursabot@rok@raulcd@wgtmac

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database by jorisvandenbossche · Pull Request #35735 · apache/arrow · GitHub
Skip to content
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database - #35735
Merged
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Jun 6, 2023
Merged
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database#35735
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Conversation
@jorisvandenbossche
@jorisvandenbosschejorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Rationale for this change
Fix up of #35395, skipping one of the tests added in that PR on Windows, because the test requires access to a tz database.
@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@jorisvandenbosschejorisvandenbossche
mentioned this pull request
May 24, 2023
 Merged

@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

Revision: 64c0d83
Submitted crossbow builds: ursacomputing/crossbow @ actions-fe7da097ff
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@pitrou
pitrou
requested a review
from rokMay 24, 2023 10:33
@pitrou
pitrou
commented
May 24, 2023

Copy link


Copy Markdown

Member
@rok does this look right? We are able to somehow use a timezone database on Windows, IIRC?
@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Yeah, the better fix would actually be to detect if the database is available, and only skip if not found on windows. Because this patch skips the tests altogether, even when it can actually work (eg on appveyor the tests pass because we download the tzdata during appveyor setup). But just to note: we currently do this plain skip for other tests requiring the database as well, so that is an existing issue with our test setup.
I assume checking if the path %USERPROFILE%\Downloads\tzdata exists might be a sufficient check at the moment (since that's the place where tz.cpp will look, and we currently don't yet allow customizing that in pyarrow).
@danepitkin
danepitkin
commented
May 24, 2023

Copy link


Copy Markdown

Member
Thanks for fixing! There are other tests in this file that also use pytest.importorskip("pytz"), should they be updated as well? It's not clear to me why this test case behaves differently.
rok
rok
reviewed
May 24, 2023
View reviewed changes
@rokrok
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

For completeness - I believe db download for windows was added by @wjones127 in #12536.
Comment threadpython/pyarrow/tests/test_scalars.py
Outdated

@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
May 24, 2023
@wjones127
wjones127
commented
May 25, 2023

Copy link


Copy Markdown

Member
For completeness - I believe db download for windows was added by @wjones127 in #12536.
That was just for C++ and R. I don't think we figured out how to do it for Python, since we need the text form of the IANA database while Python's timezone packages provide the binary form.
raulcd
raulcd
approved these changes
May 27, 2023
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

We already have 3 tests being skipped on Windows for the same reason.
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L1851
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042

I am +1 to merge
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
May 27, 2023
rok
rok
approved these changes
May 27, 2023
View reviewed changes
jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023
View reviewed changes
Comment threadpython/pyarrow/tests/test_compute.py

Comment on lines

+1862
 to 
+1864

 if sys.platform != "win32":
 # Locale-dependent formats don't match on Windows
 formats.extend(["%c", "%x", "%X"])
@jorisvandenbosschejorisvandenbosscheMay 31, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

The error on Appveyor we got was:
 @pytest.mark.skipif(sys.platform == "win32" and not util.windows_has_tzdata(),
reason="Timezone database is not installed on Windows")
def test_strftime():
times = ["2018-03-10 09:00", "2038-01-31 12:23", None]
timezones = ["CET", "UTC", "Europe/Ljubljana"]
formats = ["%a", "%A", "%w", "%d", "%b", "%B", "%m", "%y", "%Y", "%H",
"%I", "%p", "%M", "%z", "%Z", "%j", "%U", "%W", "%c", "%x",
"%X", "%%", "%G", "%V", "%u"]
for timezone in timezones:
ts = pd.to_datetime(times).tz_localize(timezone)
for unit in ["s", "ms", "us", "ns"]:
tsa = pa.array(ts, type=pa.timestamp(unit, timezone))
for fmt in formats:
options = pc.StrftimeOptions(fmt)
result = pc.strftime(tsa, options=options)
expected = pa.array(ts.strftime(fmt))
> assert result.equals(expected)
E assert False
E + where False = <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600>(<pyarrow.lib.StringArray object at 0x0000023767338830>\n[\n "Sat Mar 10 09:00:00 2018",\n "Sun Jan 31 12:23:00 2038",\n null\n])
E + where <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600> = <pyarrow.lib.StringArray object at 0x0000023767338600>\n[\n "03/10/18 09:00:00",\n "01/31/38 12:23:00",\n null\n].equals
pyarrow\tests\test_compute.py:1872: AssertionError

So it seems that we create a string like "Sat Mar 10 09:00:00 2018", but the python version we compare with gives "03/10/18 09:00:00". According to docs for %c, the former (our result) is actually correct.

But since we are checking matching results in Python in this test, just skipping the ones where those don't match.
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
May 31, 2023
@jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023

Copy link


Copy Markdown

MemberAuthor
I updated the PR to actually check if the tzdata database was present, and only skip the tests if not. Because we do actually already download the tzdb on Appveyor (not on the nightly wheel builds, so therefore the tests were failing there).

While Python doesn't yet allow you to configure the path (and use the data from the tzdata package), as mentioned above, if you manually download it to the correct location, the C++ code will already discover it (as was done for R).
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actions

This comment was marked as outdated.

Sign in to view
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actionsgithub-actionsBot
added

awaiting change review
Awaiting change review
and removed

awaiting changes
Awaiting changes
labels
Jun 6, 2023
@github-actions
github-actionsBot
commented
Jun 6, 2023

Copy link


Copy Markdown

Revision: 9720e36
Submitted crossbow builds: ursacomputing/crossbow @ actions-5e510fd809
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@jorisvandenbossche
jorisvandenbossche
merged commit c78ef0f
into
apache:mainJun 6, 2023
@jorisvandenbossche
jorisvandenbossche
deleted the

GH-35040-fix-ci

branch
June 6, 2023 15:24
@ursabot
ursabot
commented
Jun 7, 2023

Copy link


Copy Markdown

Benchmark runs are scheduled for baseline = 4ec231b and contender = c78ef0f. c78ef0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.

Conbench compare runs links:

[Failed] ec2-t3-xlarge-us-east-2

[Failed ⬇️0.77% ⬆️0.06%] test-mac-arm

[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x

[Failed ⬇️0.27% ⬆️0.0%] ursa-thinkcentre-m75q

Buildkite builds:

[Finished] c78ef0f1 ec2-t3-xlarge-us-east-2

[Finished] c78ef0f1 test-mac-arm

[Finished] c78ef0f1 ursa-i9-9960x

[Failed] c78ef0f1 ursa-thinkcentre-m75q

[Failed] 4ec231b2 ec2-t3-xlarge-us-east-2

[Failed] 4ec231b2 test-mac-arm

[Finished] 4ec231b2 ursa-i9-9960x

[Failed] 4ec231b2 ursa-thinkcentre-m75q

Supported benchmarks:

ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True

test-mac-arm: Supported benchmark langs: C++, Python, R

ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript

ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
@mattijnmattijn
mentioned this pull request
Jun 7, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rokrokrok approved these changes
@raulcdraulcdraulcd approved these changes
@wgtmacwgtmacwgtmac approved these changes
@AlenkaFAlenkaFAwaiting requested review from AlenkaFAlenkaF is a code owner
@danepitkindanepitkinAwaiting requested review from danepitkin

Assignees


No one assigned


Labels

awaiting change reviewAwaiting change reviewComponent: Python

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@jorisvandenbossche@pitrou@danepitkin@wjones127@ursabot@rok@raulcd@wgtmac

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database by jorisvandenbossche · Pull Request #35735 · apache/arrow · GitHub
Skip to content
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database - #35735
Merged
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Jun 6, 2023
Merged
GH-35040: [Python] Skip test_cast_timestamp_to_string on Windows because it requires tz database#35735
jorisvandenbossche merged 4 commits into
apache:mainfrom 
jorisvandenbossche:GH-35040-fix-ci
Conversation
@jorisvandenbossche
@jorisvandenbosschejorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

Member
Rationale for this change
Fix up of #35395, skipping one of the tests added in that PR on Windows, because the test requires access to a tz database.
@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@jorisvandenbosschejorisvandenbossche
mentioned this pull request
May 24, 2023
 Merged

@github-actions
github-actionsBot
commented
May 24, 2023

Copy link


Copy Markdown

Revision: 64c0d83
Submitted crossbow builds: ursacomputing/crossbow @ actions-fe7da097ff
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@pitrou
pitrou
requested a review
from rokMay 24, 2023 10:33
@pitrou
pitrou
commented
May 24, 2023

Copy link


Copy Markdown

Member
@rok does this look right? We are able to somehow use a timezone database on Windows, IIRC?
@jorisvandenbossche
jorisvandenbossche
commented
May 24, 2023•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Yeah, the better fix would actually be to detect if the database is available, and only skip if not found on windows. Because this patch skips the tests altogether, even when it can actually work (eg on appveyor the tests pass because we download the tzdata during appveyor setup). But just to note: we currently do this plain skip for other tests requiring the database as well, so that is an existing issue with our test setup.
I assume checking if the path %USERPROFILE%\Downloads\tzdata exists might be a sufficient check at the moment (since that's the place where tz.cpp will look, and we currently don't yet allow customizing that in pyarrow).
@danepitkin
danepitkin
commented
May 24, 2023

Copy link


Copy Markdown

Member
Thanks for fixing! There are other tests in this file that also use pytest.importorskip("pytz"), should they be updated as well? It's not clear to me why this test case behaves differently.
rok
rok
reviewed
May 24, 2023
View reviewed changes
@rokrok
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

For completeness - I believe db download for windows was added by @wjones127 in #12536.
Comment threadpython/pyarrow/tests/test_scalars.py
Outdated

@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
May 24, 2023
@wjones127
wjones127
commented
May 25, 2023

Copy link


Copy Markdown

Member
For completeness - I believe db download for windows was added by @wjones127 in #12536.
That was just for C++ and R. I don't think we figured out how to do it for Python, since we need the text form of the IANA database while Python's timezone packages provide the binary form.
raulcd
raulcd
approved these changes
May 27, 2023
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

We already have 3 tests being skipped on Windows for the same reason.
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L1851
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042
https://github.com/apache/arrow/blob/main/python/pyarrow/tests/test_compute.py#L2042

I am +1 to merge
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
May 27, 2023
rok
rok
approved these changes
May 27, 2023
View reviewed changes
jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023
View reviewed changes
Comment threadpython/pyarrow/tests/test_compute.py

Comment on lines

+1862
 to 
+1864

 if sys.platform != "win32":
 # Locale-dependent formats don't match on Windows
 formats.extend(["%c", "%x", "%X"])
@jorisvandenbosschejorisvandenbosscheMay 31, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

The error on Appveyor we got was:
 @pytest.mark.skipif(sys.platform == "win32" and not util.windows_has_tzdata(),
reason="Timezone database is not installed on Windows")
def test_strftime():
times = ["2018-03-10 09:00", "2038-01-31 12:23", None]
timezones = ["CET", "UTC", "Europe/Ljubljana"]
formats = ["%a", "%A", "%w", "%d", "%b", "%B", "%m", "%y", "%Y", "%H",
"%I", "%p", "%M", "%z", "%Z", "%j", "%U", "%W", "%c", "%x",
"%X", "%%", "%G", "%V", "%u"]
for timezone in timezones:
ts = pd.to_datetime(times).tz_localize(timezone)
for unit in ["s", "ms", "us", "ns"]:
tsa = pa.array(ts, type=pa.timestamp(unit, timezone))
for fmt in formats:
options = pc.StrftimeOptions(fmt)
result = pc.strftime(tsa, options=options)
expected = pa.array(ts.strftime(fmt))
> assert result.equals(expected)
E assert False
E + where False = <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600>(<pyarrow.lib.StringArray object at 0x0000023767338830>\n[\n "Sat Mar 10 09:00:00 2018",\n "Sun Jan 31 12:23:00 2038",\n null\n])
E + where <built-in method equals of pyarrow.lib.StringArray object at 0x0000023767338600> = <pyarrow.lib.StringArray object at 0x0000023767338600>\n[\n "03/10/18 09:00:00",\n "01/31/38 12:23:00",\n null\n].equals
pyarrow\tests\test_compute.py:1872: AssertionError

So it seems that we create a string like "Sat Mar 10 09:00:00 2018", but the python version we compare with gives "03/10/18 09:00:00". According to docs for %c, the former (our result) is actually correct.

But since we are checking matching results in Python in this test, just skipping the ones where those don't match.
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
May 31, 2023
@jorisvandenbossche
jorisvandenbossche
commented
May 31, 2023

Copy link


Copy Markdown

MemberAuthor
I updated the PR to actually check if the tzdata database was present, and only skip the tests if not. Because we do actually already download the tzdb on Appveyor (not on the nightly wheel builds, so therefore the tests were failing there).

While Python doesn't yet allow you to configure the path (and use the data from the tzdata package), as mentioned above, if you manually download it to the correct location, the C++ code will already discover it (as was done for R).
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actions

This comment was marked as outdated.

Sign in to view
@jorisvandenbossche
jorisvandenbossche
commented
Jun 6, 2023

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit -g wheel
@github-actionsgithub-actionsBot
added

awaiting change review
Awaiting change review
and removed

awaiting changes
Awaiting changes
labels
Jun 6, 2023
@github-actions
github-actionsBot
commented
Jun 6, 2023

Copy link


Copy Markdown

Revision: 9720e36
Submitted crossbow builds: ursacomputing/crossbow @ actions-5e510fd809
TaskStatus
wheel-cleanGithub Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp37-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp37-amd64Github Actions
wheel-manylinux-2-28-cp37-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp37-amd64Github Actions
wheel-manylinux-2014-cp37-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp37-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions
@jorisvandenbossche
jorisvandenbossche
merged commit c78ef0f
into
apache:mainJun 6, 2023
@jorisvandenbossche
jorisvandenbossche
deleted the

GH-35040-fix-ci

branch
June 6, 2023 15:24
@ursabot
ursabot
commented
Jun 7, 2023

Copy link


Copy Markdown

Benchmark runs are scheduled for baseline = 4ec231b and contender = c78ef0f. c78ef0f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.

Conbench compare runs links:

[Failed] ec2-t3-xlarge-us-east-2

[Failed ⬇️0.77% ⬆️0.06%] test-mac-arm

[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x

[Failed ⬇️0.27% ⬆️0.0%] ursa-thinkcentre-m75q

Buildkite builds:

[Finished] c78ef0f1 ec2-t3-xlarge-us-east-2

[Finished] c78ef0f1 test-mac-arm

[Finished] c78ef0f1 ursa-i9-9960x

[Failed] c78ef0f1 ursa-thinkcentre-m75q

[Failed] 4ec231b2 ec2-t3-xlarge-us-east-2

[Failed] 4ec231b2 test-mac-arm

[Finished] 4ec231b2 ursa-i9-9960x

[Failed] 4ec231b2 ursa-thinkcentre-m75q

Supported benchmarks:

ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True

test-mac-arm: Supported benchmark langs: C++, Python, R

ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript

ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
@mattijnmattijn
mentioned this pull request
Jun 7, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@rokrokrok approved these changes
@raulcdraulcdraulcd approved these changes
@wgtmacwgtmacwgtmac approved these changes
@AlenkaFAlenkaFAwaiting requested review from AlenkaFAlenkaF is a code owner
@danepitkindanepitkinAwaiting requested review from danepitkin

Assignees


No one assigned


Labels

awaiting change reviewAwaiting change reviewComponent: Python

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@jorisvandenbossche@pitrou@danepitkin@wjones127@ursabot@rok@raulcd@wgtmac