Skip to content

Conversation

@cdiener
Copy link
Member

@cdiener cdiener commented Apr 23, 2018

So this is an attempt to repair appveyor.

I suspect the problem is that conda has not yet updated pip to version 10. So even if you update pip with conda you get pip 9. this triggers a warning from pip to stderr which appveyor interprets as failure (wrongly so).

@cdiener
Copy link
Member Author

cdiener commented Apr 23, 2018

Ok, looks like it works.

@cdiener cdiener requested a review from Midnighter April 23, 2018 15:11
@codecov-io
Copy link

codecov-io commented Apr 23, 2018

Codecov Report

Merging #697 into devel will decrease coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #697      +/-   ##
==========================================
- Coverage   81.77%   81.66%   -0.11%     
==========================================
  Files          36       36              
  Lines        3644     3644              
  Branches      832      832              
==========================================
- Hits         2980     2976       -4     
- Misses        470      474       +4     
  Partials      194      194
Impacted Files Coverage Δ
cobra/util/version_info.py 100% <100%> (ø) ⬆️
cobra/io/sbml3.py 82.08% <0%> (-0.91%) ⬇️

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 542b0e8...58a2c0a. Read the comment docs.

Copy link
Member

@Midnighter Midnighter left a comment

Choose a reason for hiding this comment

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

Thank you for making the fixes! I've been a bit negligent of cobrapy lately but will dedicate more time to it next week.

Two inline comments.

appveyor.yml Outdated
conda install -q pip setuptools wheel numpy scipy;
pip install -r develop-requirements.txt;
pip install .
pip install --disable-pip-version-check -r develop-requirements.txt;
Copy link
Member

Choose a reason for hiding this comment

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

Rather than setting these options on each command I prefer to set an environment variable PIP_DISABLE_PIP_VERSION_CHECK=yes.

Copy link
Member

Choose a reason for hiding this comment

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

In fact, I just set this variable through the interface so you don't have to worry about it. These options can simply be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it bother if I still set the environment variable in the config? Just so that it is explicit and people could fork the config and it would still work.

Copy link
Member

Choose a reason for hiding this comment

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

Good idea, please do.

dependencies = frozenset(PKG_ORDER)
blob = dict()
for dist in pip.get_installed_distributions():
dists = [d for d in pkg_resources.working_set]
Copy link
Member

Choose a reason for hiding this comment

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

Since dists is not used for anything else can you combine these two lines to:

for dist in pkg_resources.working_set:

please?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do. This is actually taken from #696.

Copy link
Member

@Midnighter Midnighter left a comment

Choose a reason for hiding this comment

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

Thank you Christian 👍

@Midnighter Midnighter merged commit aeee5a0 into opencobra:devel Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants