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

CI: correct azure-36-locale slow name + move pyarrow #30065

Merged

Conversation

jorisvandenbossche
Copy link
Member

Follow-up on #30039

I put it in the wrong file, because the file names were swapped...

Now, here, I just renamed both files to the other name (the diff apparently cannot the rename, therefore you see a lot of changes, but I can ensure I didn't change anything except for pyarrow).
But, I could also leave the files as is and switch them in the posix.yml file ? (use the envs for the different build)

I don't know if there was something specific in either environment that was specifically added for the slow tests?

cc @datapythonista

@jorisvandenbossche jorisvandenbossche added the CI Continuous Integration label Dec 4, 2019
@jorisvandenbossche jorisvandenbossche added this to the 1.0 milestone Dec 4, 2019
@jbrockmendel
Copy link
Member

is the test failure in the py37_locale build one of the things this PR is intended to address?

@jorisvandenbossche
Copy link
Member Author

I didn't change anything about the 37 build (only 36), so no ;)
I also didn't see that one yet, thanks for pointing out. It's from the PR I merged earlier today (#29961, and which I should have updated with master after the CI changes to catch this failing test, in hindsight)

@jorisvandenbossche
Copy link
Member Author

I quickly "fixed" CI (-> #30069), will look into it tomorrow properly

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

The renaming makes sense. I saw that before, was part of #29685.

For me the way the tests are specified is still too complex to understand. So I don't really know what makes sense regarding the changes in versions. But if you know what you're doing, I'm happy with this, and hopefully we revisit all those dependency files for #29685 soon.

@jorisvandenbossche
Copy link
Member Author

But if you know what you're doing, I'm happy with this

To be clear, I explicitly don't know what I am doing ;) Because there is no guideline or docs or rationale on what is the purpose of which build or what is supposed to be tested in which build.

But, basically I didn't change anything in this PR (except for the pyarrow stuff), I just fixed the naming to be logical. The main thing I was wondering is that maybe the names weren't wrong in the first places, but we were accidentally using the wrong file in the posix.yml, and maybe I should only have changed it there (which would actually change which packages are used in the slow test build).

@jorisvandenbossche
Copy link
Member Author

For the actual issue I was trying to solve: I checked that in the Linux py36_locale build pyarrow 0.12 is installed, and the parquet tests are actually run (not listed in the skipped ones), so now it is working as intended.

@datapythonista
Copy link
Member

I was referring to unpinning all the packages. I think the renaming is the best we can do so far, at least the names match what's being tested.

@jorisvandenbossche
Copy link
Member Author

I was referring to unpinning all the packages.

To be clear, in case you are talking about this PR: I am trying to explain I didn't do any of that. The "big diff" is just because git/github couldn't handle the rename

@datapythonista
Copy link
Member

ah, I see, didn't get that. All good then.

@jorisvandenbossche
Copy link
Member Author

OK, merging then!

@jorisvandenbossche jorisvandenbossche merged commit b1c32b5 into pandas-dev:master Dec 5, 2019
@jorisvandenbossche jorisvandenbossche deleted the ci-pyarrow-correct branch December 5, 2019 17:44
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants