Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove Chainer v2 test from Travis #154

Closed
wants to merge 3 commits into from

Conversation

delta2323
Copy link
Member

Since Chainer v4 has been released, Chainer installed by CHAINER_VERSION="chainer in travis has been changed to from v4 to v3. That means Chainer v3 is not tested in the current configuration. This PR adds a test with Chainer v4 to .travis.yml.

I did not remove configuration about Chainer v2 (CHAINER_VERSION="chainer<3) because testing time in Travis is not a bottleneck in the current workflow. But I would be like opinions.

@codecov-io
Copy link

codecov-io commented Apr 23, 2018

Codecov Report

Merging #154 into master will increase coverage by 1.84%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
+ Coverage   77.54%   79.39%   +1.84%     
==========================================
  Files          79       81       +2     
  Lines        3385     3562     +177     
==========================================
+ Hits         2625     2828     +203     
+ Misses        760      734      -26

@corochann
Copy link
Member

I think building miniconda takes most of the time. Does it build miniconda 12 times?

@delta2323
Copy link
Member Author

delta2323 commented Apr 26, 2018

Does it build miniconda 12 times?

Yes, in total. The reason for using miniconda is that conda is a recommended way to install RDKit (if I remember correctly) and we can expect most users install in this way.

@corochann
Copy link
Member

I actually feel the time for building miniconda is not so short, and actually test takes some sort of time.
Do you think the overhead is acceptable?
Is there any way to make test faster, e.g., use docker image etc?

@delta2323
Copy link
Member Author

delta2323 commented May 1, 2018

I feel sometimes many jobs are queued (e.g. before releasing a new version), so it would be better to reduce testing time.

Currently we test with v2, 4, and 5, I think at least we should replace v2 with v3, if we do not want to increase the number of configurations in the CI.

@corochann
Copy link
Member

I feel if we want to support v2 - latest, we should test v2 (edge of the supporting version).

@delta2323
Copy link
Member Author

delta2323 commented May 2, 2018

An alternative way is to remove tests with Python 3.5.

@delta2323
Copy link
Member Author

delta2323 commented May 19, 2018

I remove tests with Python 3.5 as to keep testing time. It is just a workaround and we need to consider a better way.

@delta2323 delta2323 changed the title Add chainer v3 test to travis Remove Chainer v2 test from Travis Jun 21, 2018
@delta2323
Copy link
Member Author

Let me close this PR and resend it.

@delta2323 delta2323 closed this Jun 21, 2018
@mottodora mottodora added this to the Closed PRs milestone Jul 3, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants