CI: Fixing possible bugs in the CI #23727

datapythonista · 2018-11-15T21:23:27Z

I'm not sure if they are actual bugs, or I'm not understanding something. I think it's easier to discuss over a PR, so I fixed what I think it's wrong.

@jreback @TomAugspurger do you mind taking a look?

datapythonista

Added the reasons why I think things are wrong, so it's easier for you to understand my point.

datapythonista · 2018-11-15T21:24:36Z

ci/azure/linux.yml

@@ -15,7 +15,7 @@ jobs:
        CONDA_ENV: pandas
        TEST_ARGS: "--skip-slow --skip-network"

-      py36_locale:
+      py37_locale:


In CONDA_PY we have 37, as well as in the yaml file, so I assume the name is wrong.

datapythonista · 2018-11-15T21:25:15Z

ci/azure/linux.yml

@@ -27,6 +27,7 @@ jobs:
        CONDA_PY: "36"
        CONDA_ENV: pandas
        TEST_ARGS: "--only-slow --skip-network"
+        LOCALE_OVERRIDE: "zh_CN.UTF-8"


We have _locale in the name of the build, but we weren't setting LOCALE_OVERRIDE

we should be testing ZH and IT is this the missing one?

Not sure which is the build that had to use Italian, but we didn't have one for what I've seen.

Set the locale to it_IT.UTF8 here, I guess that's what we want.

datapythonista · 2018-11-15T21:25:48Z

ci/deps/azure-37-locale.yaml

@@ -18,7 +18,7 @@ dependencies:
  - pymysql
  - pytables
  - python-dateutil
-  - python=3.6*
+  - python=3.7*


In the name of the file we say it's 37, I guess the version here is wrong.

looks like this is breaking the installation of this build, as moto can't be installed with python 3.7

Seems like conda-forge/moto-feedstock#15 is failing on a dependency.

@datapythonista @TomAugspurger
Moto is supporting python 3.7. starting with 1.3.7 (the latest version), but that isn't reflected in the requirements yet: getmoto/moto#1886

It's possible to install it through pip - I did that in #23731, but then the problem is that 1.3.7 requires a boto version that will run into #23754, probably because of a moto issue: getmoto/moto#1941

datapythonista · 2018-11-15T21:26:39Z

ci/script_multi.sh

@@ -6,6 +6,7 @@ source activate pandas

 if [ -n "$LOCALE_OVERRIDE" ]; then
    export LC_ALL="$LOCALE_OVERRIDE";
+    export LANG="$LOCALE_OVERRIDE";


We have this in ci/script_single.sh but not here. I don't see why in one case should be needed and not in the other. I guess it's missing here (or could be removed there).

you could add this would prob be ok
but all of the locale tests are done in the single i think

datapythonista · 2018-11-15T21:29:13Z

ci/script_multi.sh

-    echo pytest -m "not single and slow" -v --durations=10 --junitxml=test-data-multiple.xml --strict $TEST_ARGS pandas
-    pytest      -m "not single and slow" -v --durations=10 --junitxml=test-data-multiple.xml --strict $TEST_ARGS pandas
+    echo pytest -n 2 -m "not single" -v --durations=10 --junitxml=test-data-multiple.xml --strict $TEST_ARGS pandas
+    pytest      -n 2 -m "not single" -v --durations=10 --junitxml=test-data-multiple.xml --strict $TEST_ARGS pandas


This is in script_multi.py, so I assume for the slow tests with also want to test with two processes, like in the others.

I don't think the slow in the -m is needed, as we already have the --only-slow in TEST_ARGS that is the way we call the slow tests everywhere else (and it's handled in conftest.py

you can try but this has issues before iirc

…as used instead

codecov · 2018-11-20T17:15:38Z

Codecov Report

Merging #23727 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #23727      +/-   ##
==========================================
+ Coverage   92.28%   92.29%   +<.01%     
==========================================
  Files         161      161              
  Lines       51500    51497       -3     
==========================================
+ Hits        47528    47530       +2     
+ Misses       3972     3967       -5

Flag	Coverage Δ
#multiple	`90.69% <ø> (ø)`	⬆️
#single	`42.42% <ø> (+0.11%)`	⬆️

Impacted Files	Coverage Δ
pandas/io/parsers.py	`95.29% <0%> (-0.07%)`	⬇️
pandas/core/base.py	`97.6% <0%> (-0.01%)`	⬇️
pandas/core/indexes/multi.py	`95.49% <0%> (-0.01%)`	⬇️
pandas/core/generic.py	`96.84% <0%> (-0.01%)`	⬇️
pandas/core/strings.py	`98.58% <0%> (ø)`	⬆️
pandas/core/algorithms.py	`95.11% <0%> (ø)`	⬆️
pandas/core/indexes/interval.py	`94.69% <0%> (ø)`	⬆️
pandas/core/ops.py	`94.27% <0%> (ø)`	⬆️
pandas/core/indexing.py	`93.87% <0%> (ø)`	⬆️
pandas/core/categorical.py	`100% <0%> (ø)`	⬆️
... and 18 more

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 20ae454...e1a6f89. Read the comment docs.

…ery verbose log

…of the log in travis

jreback · 2018-11-21T13:18:19Z

ci/deps/azure-37-locale.yaml

@@ -18,6 +18,9 @@ dependencies:
  - pymysql
  - pytables
  - python-dateutil
+  # XXX We should be testing ``python=3.7*`` here, but `moto` is


you can just fix this, add moto with a min version to the pip section

For what I understood, the moto version compatible with Python 3.7 is not yet released. Did I misunderstood something?

@datapythonista @jreback
Did you see my comment above #23727 (comment) ?

@datapythonista @TomAugspurger
Moto is supporting python 3.7. starting with 1.3.7 (the latest version), but that isn't reflected in the requirements yet: getmoto/moto#1886
It's possible to install it through pip - I did that in #23731, but then the problem is that 1.3.7 requires a boto version that will run into #23754, probably because of a moto issue: getmoto/moto#1941

The credentials issue would be resolved (or rather deferred) by #23731, which makes all boto tests use the s3_resource fixture, which would turn those NoCredentialsErrors into skips.

still if you can just pip install it i am not sure of the problem.

Moved to pip, let's see if tests pass. For @h-vetinari comment I understood that even pip version would make the tests fail.

…onda version is not py37 compatible

datapythonista · 2018-11-21T16:29:26Z

ci/script_multi.sh

+    # and also added to `test-data-multiple.xml`, and then printed in the log in the call to `ci/print_skipped.py`.
+    # Printing them to the log makes the log exceed the maximum size allowed by Travis and makes the build fail.
+    echo pytest -n 2 -m "not single and slow" --durations=10 --junitxml=test-data-multiple.xml --strict $TEST_ARGS pandas
+    pytest      -n 2 -m "not single and slow" --durations=10 --junitxml=test-data-multiple.xml --strict $TEST_ARGS pandas


not easy to see in the diff, but note that I added the -n 2 here, so the slow tests are being called with two processes in script_multi.sh now.

datapythonista · 2018-11-21T16:30:41Z

@jreback @TomAugspurger

Installing moto from pip seems to work. But py37_locale to use indeed Python 3.7 causes some deprecation warnings that I think we should fix:

AssertionError: Caused unexpected warning(s): [('DeprecationWarning', DeprecationWarning('time.clock has been deprecated in Python 3.3 and will be removed from Python 3.8: use time.perf_counter or time.process_time instead'), '/home/vsts/miniconda3/envs/pandas/lib/python3.7/site-packages/xlrd/book.py', 74), ('DeprecationWarning', DeprecationWarning('time.clock has been deprecated in Python 3.3 and will be removed from Python 3.8: use time.perf_counter or time.process_time instead'), '/home/vsts/miniconda3/envs/pandas/lib/python3.7/site-packages/xlrd/book.py', 89), ('DeprecationWarning', DeprecationWarning('time.clock has been deprecated in Python 3.3 and will be removed from Python 3.8: use time.perf_counter or time.process_time instead'), '/home/vsts/miniconda3/envs/pandas/lib/python3.7/site-packages/xlrd/book.py', 130)].

Can you confirm, and I'll create a separate issue for it?

@TomAugspurger can you check if regardless of the failure because of the warnings, the changes are made are really fixing typos, or if I misunderstood something

h-vetinari · 2018-11-21T16:35:57Z

@datapythonista

Moved to pip, let's see if tests pass. For @h-vetinari comment I understood that even pip version would make the tests fail.

FYI, the tests I was talking about got skipped because s3fs was not installed. That's fine if you just want moto without the boto tests. If you do want the boto tests, you'll need to add s3fs to the conda environment (and then you'll run into #23754 for pandas/tests/io/json/test_compression.py::test_with_s3_url and pandas/tests/io/test_excel.py::TestXlrdReader::test_read_from_s3_url; the other s3 tests would be silently skipped in the except: pytest.skip clause of the s3_resource fixture)

TomAugspurger · 2018-11-21T21:36:55Z

But py37_locale to use indeed Python 3.7 causes some deprecation warnings that I think we should fix:

I think you can safely add a filter for that specific warning, since it's coming from xlrd and not us. The only reason it's failing our build is because it's bubbling up when we're expecting a different warning.

pep8speaks · 2018-11-21T22:45:54Z

Hello @datapythonista! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/tests/io/test_excel.py !

datapythonista · 2018-11-21T23:57:25Z

Added the ignores for the warnings, all green now testing with Python 3.7, @jreback @TomAugspurger let me know if there is any other change required.

jreback

looks good.

jreback · 2018-11-23T03:16:24Z

pandas/tests/io/test_excel.py


        # usecols as int
        with tm.assert_produces_warning(FutureWarning,
                                        check_stacklevel=False):
-            df2 = self.get_exceldf("test1", ext, "Sheet2", skiprows=[1],
-                                   index_col=0, usecols=3)
+            with warnings.catch_warnings():


can you put this in a decorator / context manger so its not repeated so much

datapythonista · 2018-11-23T10:35:45Z

good suggestion @jreback, made the warning ignore a context manager, all green

jreback · 2018-11-23T14:34:06Z

pandas/tests/io/test_excel.py

+@contextlib.contextmanager
+def ignore_xlrd_time_clock_warning():
+    """
+    Context manager to ignore warnings raised by the xlrd library,


hmm, is this still present in 1.0.0 which is now our min version?

it happens in 1.1.0 (the latest), which is the version I have installed, so I think these warning ignores are needed for now

jreback · 2018-11-24T04:59:01Z

thanks!

Fixing possible bugs in the CI

57aca9a

datapythonista added Bug CI Continuous Integration Dependencies Required and optional dependencies labels Nov 15, 2018

datapythonista commented Nov 15, 2018

View reviewed changes

datapythonista added 2 commits November 20, 2018 17:12

Moving Python 3.7 test to 3.6, as moto is failing on 3.7 at the moment

08ccb80

Testing with Italian locale (was supposed to be tested, but Chinese w…

cfc89a5

…as used instead

datapythonista added 4 commits November 20, 2018 19:56

Restoring -m 'not single and slow' to see if removing it caused the v…

c8d20b1

…ery verbose log

callng multi/slow tests in a consistent way with the rest

9718fd5

Reducing verbosity of slow tests, to avoid reaching the maximum size …

b3f5744

…of the log in travis

restoring -m slow so the build passes, and added comment explaining why

22b8fc2

jreback requested changes Nov 21, 2018

View reviewed changes

Moving azure 37 build back to 3.7, and installing moto from pip, as c…

5afef71

…onda version is not py37 compatible

datapythonista commented Nov 21, 2018

View reviewed changes

Filtering xlrd warnings in Python 3.7

6f17fe5

Merging from master

23d0c17

jreback requested changes Nov 23, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Nov 23, 2018

datapythonista added 2 commits November 23, 2018 08:45

Merge remote-tracking branch 'upstream/master' into ci_bugs

d08c0a8

Using a context manager to ignore xlrd warnings

e1a6f89

jreback approved these changes Nov 23, 2018

View reviewed changes

jreback reviewed Nov 23, 2018

View reviewed changes

jreback merged commit f2839fe into pandas-dev:master Nov 24, 2018

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

CI: Fixing possible bugs in the CI (pandas-dev#23727)

d578f9d

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

CI: Fixing possible bugs in the CI (pandas-dev#23727)

39e41dc

simonjayhawkins mentioned this pull request Aug 15, 2020

BLD: bump xlrd min version to 1.2.0 #35728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: Fixing possible bugs in the CI #23727

CI: Fixing possible bugs in the CI #23727

datapythonista commented Nov 15, 2018

datapythonista left a comment

datapythonista Nov 15, 2018

datapythonista Nov 15, 2018

jreback Nov 15, 2018

datapythonista Nov 20, 2018

datapythonista Nov 15, 2018

datapythonista Nov 15, 2018

TomAugspurger Nov 16, 2018

h-vetinari Nov 19, 2018

datapythonista Nov 15, 2018

jreback Nov 15, 2018

datapythonista Nov 15, 2018

jreback Nov 15, 2018

codecov bot commented Nov 20, 2018 •

edited

Loading

jreback Nov 21, 2018

datapythonista Nov 21, 2018

h-vetinari Nov 21, 2018

jreback Nov 21, 2018

datapythonista Nov 21, 2018

datapythonista Nov 21, 2018

datapythonista commented Nov 21, 2018

h-vetinari commented Nov 21, 2018

TomAugspurger commented Nov 21, 2018

pep8speaks commented Nov 21, 2018

datapythonista commented Nov 21, 2018

jreback left a comment

jreback Nov 23, 2018

datapythonista commented Nov 23, 2018

jreback Nov 23, 2018

datapythonista Nov 24, 2018

jreback commented Nov 24, 2018

CI: Fixing possible bugs in the CI #23727

CI: Fixing possible bugs in the CI #23727

Conversation

datapythonista commented Nov 15, 2018

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

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

Choose a reason for hiding this comment

codecov bot commented Nov 20, 2018 • edited Loading

Codecov Report

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

datapythonista commented Nov 21, 2018

h-vetinari commented Nov 21, 2018

TomAugspurger commented Nov 21, 2018

pep8speaks commented Nov 21, 2018

datapythonista commented Nov 21, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Nov 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Nov 24, 2018

codecov bot commented Nov 20, 2018 •

edited

Loading