search

HDI-Project / ATM

Auto Tune Models - A multi-tenant, multi-data system for automated machine learning (model selection and tuning).
https://hdi-project.github.io/ATM/
MIT License
527 stars 141 forks source link

Fix pandas.Series.items does not exist in py2 #86

Closed micahjsmith closed 6 years ago

micahjsmith commented 6 years ago

The fact that the items attribute exists in py3 is just a coincidence. It should be iteritems. See https://pandas.pydata.org/pandas-docs/stable/generated/pandas.Series.iteritems.html.

This closes #85

codecov-io commented 6 years ago

Codecov Report

Merging #86 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #86   +/-   ##
=======================================
  Coverage   74.31%   74.31%           
=======================================
  Files          12       12           
  Lines        1234     1234           
=======================================
  Hits          917      917           
  Misses        317      317
Impacted Files Coverage Δ
atm/database.py 73.77% <100%> (ø) :arrow_up:

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 959bdc2...aa9196d. Read the comment docs.

micahjsmith commented 6 years ago

A followup -- I'm perplexed why this wasn't showing up in the CircleCI tests. You can see the output of recent test runs in that it passes py27 testing, but that is on pandas 0.22.0 which has added backwards compatibility for items. Does CircleCI not install versions as specified in requirements.txt?

bcyphers commented 6 years ago

Re: CircleCI -- it runs make installdeps, which runs pip install -e ., which in turn installs dependencies as defined in setup.py's install_requires list. While requirements.txt specifies exact versions, install_requires defines a bunch of minimum versions. This is best practice, according to https://packaging.python.org/discussions/install-requires-vs-requirements/:

It is not considered best practice to use install_requires to pin dependencies to specific versions, or to specify sub-dependencies (i.e. dependencies of your dependencies). This is overly-restrictive, and prevents the user from gaining the benefit of dependency upgrades.

Anyway, that's why CircleCI always tests with the latest version of the dependencies, while users installing from requirements.txt don't. To address this, we could change the requirements newer versions -- I agree that we shouldn't prevent users from using the latest versions of other libraries, especially because we want atm to be used alongside other machine learning tools.

bcyphers commented 6 years ago

For now, I'm going to update the version of pandas in requirements.txt to 0.22 and merge.