Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix deprecation warning. #6716

Merged
merged 3 commits into from
Aug 22, 2019
Merged

Fix deprecation warning. #6716

merged 3 commits into from
Aug 22, 2019

Conversation

rpgoldman
Copy link
Contributor

Change the import of Iterable from collections.Iterable to collections.abc.Iterable to future-proof against Python 3.7.8.

Caught this warning when running the PyMC3 tests.

@nouiz
Copy link
Member

nouiz commented Aug 19, 2019

Thanks for the PR, but this break our build bot. As Theano ins't developed anymore, we won't change the requirement. Can you make this work with the older Python version that we still support? Python 2.7, 3.4-3.6

@rpgoldman
Copy link
Contributor Author

@nouiz OK, working on it. I'm afraid I don't have a full setup for Theano development here, so I'm relying on your build bot to test. If this works, you can squash merge it, otherwise I will try again until I get something that passes tests.

@rpgoldman
Copy link
Contributor Author

@nouiz I'm confused by the test results: when I look at https://travis-ci.org/Theano/Theano/jobs/573893290 I see that the test failed, but AFAICT it's because the code isn't formatted to comply with flake8, and that problem isn't limited to my file -- it's uniform across the code base. So is this a real failure, or should this test be revised or dropped? Alternatively, is there a chance that flake8 has changed, and that's why this is happening?
Also, the Python 3.4 doc tests seem to fail because of something going on in Pygments, that is unrelated to my little fix -- it's also seen in Adrian's pull request.
Maybe it's time to force a rebuild of master and see if the tests themselves have issues since last run 3 months ago?

@nouiz
Copy link
Member

nouiz commented Aug 21, 2019

we fix the pep8 and flake8 version for tests. So this shouldn't be a problem. But there is one:)
I started a build on the master of Theano while having deleted travis cache:
https://travis-ci.org/Theano/Theano/builds/574872469

@nouiz
Copy link
Member

nouiz commented Aug 21, 2019

master failed. I do not understand what changes could have caused this.
Here is a PR that try to fix master: #6717
Mostly I just ignore the error that we didn't enforced before.

@rpgoldman
Copy link
Contributor Author

@nouiz

master failed. I do not understand what changes could have caused this.
Here is a PR that try to fix master: #6717
Mostly I just ignore the error that we didn't enforced before.

There's also something bad going on testing with python 3.4:

Pygments requires Python '>=2.7, !=3.0., !=3.1., !=3.2., !=3.3., !=3.4.*' but the running Python is 3.4.5
The command "pip install flake8-future-import parameterized sphinx_rtd_theme" failed and exited with 1 during .

We should probably move this discussion to a separate issue where it's easier to find.

Note that I have been discussing this with @twiecki and he also tried travis, which now failed completely on master.

@nouiz
Copy link
Member

nouiz commented Aug 21, 2019

I fixed the master. Can you rebase this PR?

Change the import of `Iterable` from `collections.Iterable` to `collections.abc.Iterable` to future-proof against Python 3.7.8.
@rpgoldman
Copy link
Contributor Author

@nouiz OK, rebased and force-pushed. Note that the history of this PR is uninteresting, so if it looks good, you should just squash-merge it...

@nouiz
Copy link
Member

nouiz commented Aug 22, 2019

Thanks. Merging.

@nouiz nouiz merged commit 3be13b2 into Theano:master Aug 22, 2019
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.

2 participants