Use bash on windows #27195

alimcmaster1 · 2019-07-02T21:59:51Z

passes git diff upstream/master -u -- "*.py" | flake8 --diff
towards CI: Use bash for Windows scripts on Azure #26344

As per travis logs:
https://travis-ci.org/pandas-dev/pandas/jobs/552444580

ccache: /usr/bin/ccache
source deactivate
DeprecationWarning: 'source deactivate' is deprecated. Use 'conda deactivate'.

As of conda 4.6 the preferred way of using an environment is to use conda activate (env).
source activate will eventually be deprecated

(only remaining place to do this is in code_checks.yml -> I will do this in a follow up for ease of review)

This make the steps in the windows.yml and posix.yml file very similar and we can potentially merge these.
Use run_tests.sh on windows this will mean we now tests with pytest -n 2 and pytest -n 1 on windows - now consistent with the linux builds

cc. @TomAugspurger @datapythonista

jreback · 2019-07-02T22:01:08Z

sure

jorisvandenbossche · 2019-07-08T13:32:09Z

@alimcmaster1 I was trying a few days ago something similar for geopandas (although for appveyor), and needed some PATH manipulations in the end to get this working (with the help of @msarahan) , see geopandas/geopandas#1029. It might be that something similar is needed here, if the conda init bash does not work on CI.

alimcmaster1 · 2019-07-08T16:19:02Z

Awesome thanks for the pointer @jorisvandenbossche

jbrockmendel · 2019-07-16T19:33:26Z

@jorisvandenbossche is this close to being ready?

alimcmaster1 · 2019-07-16T20:03:16Z

@jbrockmendel - still needs a bit of work. CI is currently only passing on windows. I will take another look

WillAyd · 2019-08-28T17:07:18Z

Doesn't look like build logs are still available but does the below link from Azure not solve the issue?

https://docs.microsoft.com/en-us/azure/devops/pipelines/languages/anaconda?view=azure-devops&tabs=ubuntu-16-04#add-conda-to-your-system-path

alimcmaster1 · 2019-08-28T20:06:39Z

Thanks @WillAyd will give that a try

WillAyd · 2019-09-02T20:16:14Z

ci/azure/posix.yml

@@ -56,15 +56,20 @@ jobs:
  steps:
    - script: |
        if [ "$(uname)" == "Linux" ]; then sudo apt-get install -y libc6-dev-i386 $EXTRA_APT; fi
-        echo '##vso[task.prependpath]$(HOME)/miniconda3/bin'
+        echo '##vso[task.setvariable variable=MINICONDA_DIR]$(HOME)/miniconda3'


I think this needs to be executed by bash and not script (which on windows is just cmd.exe). Not sure if there is an overarching reason for this and similar blocks to be script but as mentioned changing to bash might fix the issue remaining

WillAyd · 2019-09-07T19:37:16Z

ci/azure/windows.yml

        conda env create -q --file ci\\deps\\azure-windows-$(CONDA_PY).yaml
      displayName: 'Create anaconda environment'
    - script: |
-        call activate pandas-dev
-        call conda list
+        call conda.bat activate pandas-dev


Why not just change all of the script commands to bash and then just use without call? Right now the usage looks mixed so would make things consistent and easier to read I think for non-Windows people

changed all of windows.yml file script steps to use bash instead

alimcmaster1 · 2019-09-08T13:20:52Z

ci/azure/windows.yml

@@ -18,22 +18,26 @@ jobs:

  steps:
    - powershell: |
-        Write-Host "##vso[task.prependpath]$env:CONDA\Scripts"
-        Write-Host "##vso[task.prependpath]$HOME/miniconda3/bin"


Correct this to miniconda dir which is $env:CONDA

WillAyd · 2019-09-08T19:56:11Z

ci/azure/windows.yml

-        call activate
-        conda env create -q --file ci\\deps\\azure-windows-$(CONDA_PY).yaml
+    - bash: |
+        source $MINICONDA_DIR/etc/profile.d/conda.sh


Is this necessary with the vso task above?

WillAyd · 2019-09-08T19:56:36Z

ci/azure/windows.yml

+        conda update -q -y -n base conda
+      displayName: 'Update conda'
+    - bash: |
+        source $MINICONDA_DIR/etc/profile.d/conda.sh


Same question here; I think the task already puts conda on the path so no need to run this script (assuming that's what it does)

i found this was necessary too - otherwise conda complains you need to run conda init. Let me double check this though. sourcing conda.sh here seemed to handle everything nicely

confirmed I definitely need this otherwise we get:

CommandNotFoundError: Your shell has not been properly configured to use 'conda activate'. To initialize your shell, run $ conda init <SHELL_NAME> Currently supported shells are: - bash - fish - tcsh - xonsh - zsh - powershell See 'conda init --help' for more information and options.

alimcmaster1 · 2019-09-14T00:38:51Z

This is almost over the line - I will update the windows CI to use run_tests.sh so its consistent with Linux CI

alimcmaster1 · 2019-09-14T17:55:35Z

@WillAyd i've fixed this up + edited description highlighting what this hopes to achieve mind taking a look?

datapythonista

I'm a bit unsure on this... I think changing source deactivate by conda deactivate is trivial, and fixes the warning.

Didn't check the windows build in detail, but I don't think in Linux there is any warning on using source activate, and we are significantly increasing the complexity of the build to make it a conda activate.

I'd open separate PRs, as there are surely good changes here. But replacing source activate pandas-dev by source $MINICONDA_DIR/etc/profile.d/conda.sh && conda activate pandas-dev doesn't seem reasonable to me.

datapythonista · 2019-09-15T13:19:27Z

ci/azure/windows.yml


      py37_np141:
        ENV_FILE: ci/deps/azure-windows-37.yaml
        CONDA_PY: "37"
+        PATTERN: "not slow and not network"


Is this related? If not, I think we should have it in a separate PR.

Okay agree - let me factor this out to a separate PR. This was part of the work to move the windows CI scripts to use bash. (FYI I'm away from my keyboard for a few weeks - but will work on this when I return)

can you factor this out

WillAyd · 2019-09-16T02:28:24Z

But replacing source activate pandas-dev by source $MINICONDA_DIR/etc/profile.d/conda.sh && conda activate pandas-dev doesn't seem reasonable to me.

Yea agreed on that. So doesn't using bash instead of script get rid of the need for the former? I thought the point of the VSO task was to enable simple conda usage without source so long as bash is used. I'm also not sure there's a point to using script / cmd.exe on Windows

alimcmaster1 · 2019-09-29T20:15:10Z

Going to wait till #28531 is merged then I will open a separate PR to update windows.yml to use bash in the steps opposed to script. As per #26344. Unless any dev's disagree?

datapythonista · 2019-09-30T17:05:43Z

Sounds good thanks @alimcmaster1

jreback · 2019-10-01T16:56:26Z

.travis.yml

-  - echo "after_script done"
+  - conda activate pandas-dev && pushd /tmp && python -c "import pandas; pandas.show_versions();" && popd
+  - ci/print_skipped.py
+  - echo "after_script done"


need a newline

jreback · 2019-10-01T16:56:50Z

ci/azure/posix.yml

        python ci/print_skipped.py
-      displayName: 'Print skipped tests'
+      displayName: 'Print skipped tests'


add a newline

jreback · 2019-10-01T16:57:03Z

ci/azure/windows.yml


      py37_np141:
        ENV_FILE: ci/deps/azure-windows-37.yaml
        CONDA_PY: "37"
+        PATTERN: "not slow and not network"


can you factor this out

jreback · 2019-10-18T21:31:19Z

can you merge master

alimcmaster1 · 2019-10-18T23:03:21Z

Will do - waiting for @datapythonista to merge his PR first

jbrockmendel · 2019-10-21T01:31:44Z

waiting for datapythonista to merge his PR first

can you rebase here anyway?

jbrockmendel · 2019-11-01T19:41:15Z

@alimcmaster1 can you pls rebase?

jbrockmendel · 2019-11-02T00:11:40Z

@alimcmaster1 it looks like you just pushed, but GH still says there are conflicts

alimcmaster1 · 2019-11-02T11:11:03Z

@jbrockmendel sorry for the delay updated. We should wait till #28531 is merged as in my PR here we are currently calling pytest with n = 2 and n=1 on windows where as the current behaviour just calls:
pytest -m "not slow and not network" --junitxml=test-data.xml pandas -n 2 -r sxX --strict --durations=10 %*

This change means the steps in both posix.yml and windows.yml should be similar. (and we can potentially merge)

…nda-warning

alimcmaster1 · 2019-11-17T17:38:24Z

Closing as the scope of this changed slight. Will summarise and open fresh PR - to make this easier to review.

Thanks all for the comments

datapythonista · 2019-11-17T17:44:44Z

Also worth considering that we can move things to GitHub Actions

jreback added the Build Library building on various platforms label Jul 2, 2019

jreback added this to the 0.25.0 milestone Jul 2, 2019

jreback removed this from the 0.25.0 milestone Jul 3, 2019

alimcmaster1 closed this Aug 18, 2019

alimcmaster1 reopened this Aug 18, 2019

WillAyd reviewed Sep 2, 2019

View reviewed changes

WillAyd reviewed Sep 7, 2019

View reviewed changes

alimcmaster1 commented Sep 8, 2019

View reviewed changes

WillAyd reviewed Sep 8, 2019

View reviewed changes

alimcmaster1 changed the title ~~WIP: Remove conda warning~~ Remove conda warning + Use bash on windows Sep 14, 2019

datapythonista requested changes Sep 15, 2019

View reviewed changes

jreback requested changes Oct 1, 2019

View reviewed changes

Use bash on windows ci

4c790b5

alimcmaster1 changed the title ~~Remove conda warning + Use bash on windows~~ Use bash on windows Nov 2, 2019

Use bash on windows ci

0ec3fc4

alimcmaster1 added 2 commits November 17, 2019 17:05

Merge remote-tracking branch 'remotes/upstream/master' into mcmali-co…

898505b

…nda-warning

Make consistent with posix.yml

b1fb57d

alimcmaster1 closed this Nov 17, 2019

alimcmaster1 mentioned this pull request Nov 17, 2019

CI: Use bash for windows script on azure #29674

Merged

alimcmaster1 deleted the mcmali-conda-warning branch December 25, 2019 20:35

Use bash on windows #27195

Use bash on windows #27195

Conversation

alimcmaster1 commented Jul 2, 2019 • edited Loading

jreback commented Jul 2, 2019

jorisvandenbossche commented Jul 8, 2019

alimcmaster1 commented Jul 8, 2019

jbrockmendel commented Jul 16, 2019

alimcmaster1 commented Jul 16, 2019

WillAyd commented Aug 28, 2019

alimcmaster1 commented Aug 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alimcmaster1 commented Sep 14, 2019

alimcmaster1 commented Sep 14, 2019

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented Sep 16, 2019

alimcmaster1 commented Sep 29, 2019

datapythonista commented Sep 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Oct 18, 2019

alimcmaster1 commented Oct 18, 2019

jbrockmendel commented Oct 21, 2019

jbrockmendel commented Nov 1, 2019

jbrockmendel commented Nov 2, 2019

alimcmaster1 commented Nov 2, 2019

alimcmaster1 commented Nov 17, 2019

datapythonista commented Nov 17, 2019

alimcmaster1 commented Jul 2, 2019 •

edited

Loading