search

civisanalytics / civis-python

Civis API Python Client
BSD 3-Clause "New" or "Revised" License
34 stars 26 forks source link

MAINT Configure Windows CI using AppVeyor #258

Closed jacksonllee closed 6 years ago

jacksonllee commented 6 years ago

IT support has turned on AppVeyor for CI in Windows. This PR configures that, based on what we already have for Circle CI.

Resolves #10.

jacksonllee commented 6 years ago

@keithing I've defeated AppVeyor! Good news is that only changes in tests are needed for Windows compatibility. It's mostly about not using tempfile.NamedTemporaryFile. I've left comments for other changes. This PR is ready for review.

jacksonllee commented 6 years ago

Hmm, I've made changes according to Stephen's suggestions, but now the builds fail for Python 3.5 and 3.6 (for both Linux and Windows builds). For these two Python versions, it looks like we're using whatever the latest version of pandas is available from PyPI. pandas v0.23.1 was released just 9 hours ago -- either it is broken, or at least it breaks our builds.

Indeed, all builds passed with pandas v0.23.0; see the successful Linux builds triggered by 9fbb4be committed yesterday.

Back to the most recent build failures for Python 3.5 and 3.6, it looks like when _stash_dataframe_as_csv from civis/ml/_model.py calls df.to_csv, the under-the-hood pandas's CSVFormatter object calls the save method, which triggers AttributeError: '_io.TextIOWrapper' object has no attribute 'getvalue'.

I'll come back to this PR later in the day.

stephen-hoover commented 6 years ago

This test failure was caused by https://github.com/pandas-dev/pandas/pull/21300 . I'm not sure if it represents a bug on our end or just a change in the parameter options available for to_csv. I think we should fix _stash_dataframe_as_csv in a different PR so that it works with the latest version of pandas.

jacksonllee commented 6 years ago

259 fixes _stash_dataframe_as_csv. When that PR is merged, I'll rebase here.

jacksonllee commented 6 years ago

I'm sorting out the repo / AppVeyor settings with IT support. Sit tight, everyone.

jacksonllee commented 6 years ago

We're all set. All builds for both Linux and Windows have passed. This PR is ready for review again.