Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert changes to test regexes #25874

Merged
merged 1 commit into from
Mar 26, 2019
Merged

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Mar 26, 2019

Sorry, something went wrong.

@WillAyd
Copy link
Member Author

WillAyd commented Mar 26, 2019

Reverted to state before #25752

@codecov
Copy link

codecov bot commented Mar 26, 2019

Codecov Report

Merging #25874 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #25874   +/-   ##
=======================================
  Coverage   91.48%   91.48%           
=======================================
  Files         175      175           
  Lines       52881    52881           
=======================================
  Hits        48376    48376           
  Misses       4505     4505
Flag Coverage Δ
#multiple 90.04% <ø> (ø) ⬆️
#single 41.83% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51c6a05...097a67f. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Mar 26, 2019

Codecov Report

Merging #25874 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #25874   +/-   ##
=======================================
  Coverage   91.48%   91.48%           
=======================================
  Files         175      175           
  Lines       52881    52881           
=======================================
  Hits        48376    48376           
  Misses       4505     4505
Flag Coverage Δ
#multiple 90.04% <ø> (ø) ⬆️
#single 41.83% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51c6a05...097a67f. Read the comment docs.

@WillAyd WillAyd changed the title Revert changed to test regexes Revert changes to test regexes Mar 26, 2019
Copy link
Contributor

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

I find it curious that this passes - I've had persistent errors with the previous regexes. Sorry for breaking things.

I should add that I find the perfect matching of error messages to be a somewhat futile excercise, but I'm also guilty of that because in trying to be as correct as possible, I introduced the switch _np_version_under1p14 which I observed. The failure in #25867 shows that this was not the correct distinction between the two parts.

In any case, the regexes I wrote are a true superset of the ones they replaced, and I think this should be kept in case the errors I encountered return. For this reason, I'd like to just remove the switch and turn it into a regex-or-clause.

@@ -228,11 +228,9 @@ def test_complex_sorting(self):
# gh 12666 - check no segfault
x17 = np.array([complex(i) for i in range(17)], dtype=object)

msg = (r"unorderable types: {0} [<>] {0}".format(r"complex\(\)")
if _np_version_under1p14 else
Copy link
Contributor

Choose a reason for hiding this comment

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

better to just replace if _np_version_under1p14 else with "|"

@@ -414,13 +413,10 @@ def test_mixed_integer_from_list(self):
def test_unsortable(self):
# GH 13714
arr = np.array([1, 2, datetime.now(), 0, 3], dtype=object)
msg = (r"unorderable types: ({0} [<>] {1}|{1} [<>] {0})".format(
r"int\(\)", r"datetime\.datetime\(\)") # noqa: E126
if _np_version_under1p14 else
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@gfyoung gfyoung added Build Library building on various platforms CI Continuous Integration labels Mar 26, 2019
@jreback jreback added this to the 0.25.0 milestone Mar 26, 2019
@jreback
Copy link
Contributor

jreback commented Mar 26, 2019

@h-vetinari yeah not really sure why this fails the wheel builder but passes here.

@jreback jreback merged commit df6b2cd into pandas-dev:master Mar 26, 2019
@h-vetinari
Copy link
Contributor

@jreback
I had an open review request on this PR. Can you please wait with merging in such cases until it has at least been discussed?

@jreback
Copy link
Contributor

jreback commented Mar 26, 2019

@h-vetinari this was a straight revert and causing the CI to fail for several days. You should be familiar with how I work by now. We fix things step by step incrementally with orthogonal changes. you are welcome to propose changes if you want.

@WillAyd WillAyd deleted the revert-regex branch March 26, 2019 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BLD/CI: wheel building is failing
4 participants