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

Removing testing and support for Python 2.6 #322

Merged
merged 12 commits into from
Apr 26, 2018
Merged

Removing testing and support for Python 2.6 #322

merged 12 commits into from
Apr 26, 2018

Conversation

whart222
Copy link
Member

@whart222 whart222 commented Jan 25, 2018

Fixes #295

Summary/Motivation:

We do not have users actively downloading for Python 2.6, and it's becoming increasingly difficult to support testing for Python2.6 as packages that Pyomo relies on drop support for this version.

Changes proposed in this PR:

This PR does not include changes in the Pyomo source tree that
are not backwards compatible, but I expect that subsequent commits
will begin to introduce changes that are not compatible with Python 2.6.

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I are authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

This commit does not include changes in the Pyomo source tree that
are not backwards compatible, but I expect that subsequent commits
will begin to introduce changes that are not compatible with Python 2.6.
@jsiirola
Copy link
Member

@whart222: What is the motivation for this? Apart from some syntatical sugar that @ghackebeil mentions in #295, I don't see a real driver for this. I am not saying that we shouldn't do it, but I don't see a reason to bypass the developer meetings and rush this into 5.4. I would rather see this discussed and nominated for 5.5.

@whart222
Copy link
Member Author

@jsiirola New test failures have cropped up on the master branch that reflect additional package dependencies that are impacting our testing. This is the real driver for this change.

The PR is minimal, so I thought it was worth moving forward with. I agree that we should confirm that there isn't a blocker, but I don't know of any.

Finally, note that this PR does not eliminate support for Python 2.6, but rather eliminates testing for Python 2.6 compatibility.

@jsiirola
Copy link
Member

You are referring to the openpyxl release that installs in 2.6 but contains a syntax error? That is trivially fixed in our check for if openpyxl is available (master has been updated).

I will point out that if we remove testing for 2.6 (especially if it is because tests are failing), we are effectively immediately removing support for 2.6.

@whart222
Copy link
Member Author

I agree that we're eliminating support. I meant to say that this PR does not immediately eliminate compatibility with Python 2.6 (though I think that will happen eventually).

@whart222
Copy link
Member Author

Yes, I was referring to openpyxl. I've already fixed several such build failure issues. :(

@codecov-io
Copy link

codecov-io commented Jan 25, 2018

Codecov Report

Merging #322 into master will decrease coverage by 0.38%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #322      +/-   ##
==========================================
- Coverage   66.36%   65.97%   -0.39%     
==========================================
  Files         487      487              
  Lines       63180    63180              
==========================================
- Hits        41928    41686     -242     
- Misses      21252    21494     +242
Impacted Files Coverage Δ
pyomo/solvers/plugins/solvers/GAMS.py 48.09% <0%> (-29.38%) ⬇️
pyomo/core/plugins/data/sheet.py 57.97% <0%> (-11.6%) ⬇️
pyomo/core/kernel/component_piecewise/util.py 95.69% <0%> (-4.31%) ⬇️
...contrib/preprocessing/plugins/detect_fixed_vars.py 96.55% <0%> (-3.45%) ⬇️
pyomo/scripting/pyomo_parser.py 70% <0%> (-2%) ⬇️
pyomo/gdp/plugins/cuttingplane.py 25.24% <0%> (-1.95%) ⬇️
pyomo/opt/results/solution.py 91.15% <0%> (-1.37%) ⬇️
pyomo/core/base/block.py 84.84% <0%> (-1.05%) ⬇️
pyomo/core/plugins/data/db_table.py 58.42% <0%> (-0.85%) ⬇️
pyomo/gdp/plugins/chull.py 89.93% <0%> (-0.61%) ⬇️
... and 8 more

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 fd2bd02...5d883a2. Read the comment docs.

@hugovk
Copy link

hugovk commented Jan 25, 2018

I would vote for making a clean break and eliminating both testing and support for Python 2.6 at the same time.

@carldlaird carldlaird self-requested a review February 27, 2018 20:57
@blnicho blnicho added the testing_and_ci Any issues related to testing and/or continuous integration (e.g., github, travis, etc.) label Mar 1, 2018
This allows Python 2.7.x and versions greater than 3.5.x
@whart222
Copy link
Member Author

@jsiirola @carldlaird Now that we've finalized the 5.5 release, I think we're ready to process this PR. Please review.

We are no longer testing 3.4, so we should not advertise
support for 3.4
@ghackebeil
Copy link
Member

Perhaps we should add a line to the README that lists the Python versions we support. The expr_dev branch could add PyPy to that line.

@whart222
Copy link
Member Author

@ghackebeil The versions supported are documented in the setup.py file, which is used to configure the PyPI pages. This seems like the best place to document this other than the online documentation. So perhaps the other TODO here is to update the online docs. I'll do that shortly.

As discussed in the PR, we to include installation instructions in
our documentation, including a summary of supported versions.
@jsiirola
Copy link
Member

@whart222: Why did you add a3fb501? We are still testing 3.4 on both Jenkins and Travis. Is this PR now advocating dropping 2.6 AND 3.4?

@whart222
Copy link
Member Author

Yes. I'm pretty sure we omitted Python 3.4 tests recent due to performance issues. Right?

If not, then yes I'd strongly recommend deprecating 3.4 as well. The conda builds only support 2.7, 3.5 and 3.6. I've regularly seen performance issues with tests. And when developing the expression system I've seen weird test failures on 3.4 that I haven't been able to explain.

@jsiirola
Copy link
Member

No - as I said, 3.4 (and for that matter until this PR is approved, 2.6) are still "officially" supported by Pyomo and are tested on Travis and Jenkins.

@whart222
Copy link
Member Author

NOTE: We can ignore the appveyor build failures in this PR. They are resolved in PR #438

@whart222 whart222 requested review from carldlaird and removed request for carldlaird and jsiirola April 22, 2018 16:17
Copy link
Member

@carldlaird carldlaird left a comment

Choose a reason for hiding this comment

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

Only one question - see comment in code.
Also, I don't see successful tests. After successful tests - happy to merge.

@@ -12,11 +12,7 @@
import sys
import os
import subprocess
try:
Copy link
Member

Choose a reason for hiding this comment

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

Do we know that this was only for python 2.6? (i.e., is the comment correct?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so.

carldlaird
carldlaird previously approved these changes Apr 24, 2018
Copy link
Member

@carldlaird carldlaird left a comment

Choose a reason for hiding this comment

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

I approve once the tests are shown to pass.

Pyomo currently supports the following versions of Python:

* CPython: 2.7, 3.5, 3.6

Copy link
Member

Choose a reason for hiding this comment

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

@whart222 you're missing 3.4 in this list

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@jsiirola jsiirola left a comment

Choose a reason for hiding this comment

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

See comments

.. doctest::
Pyomo currently supports the following versions of Python:

* CPython: 2.7, 3.5, 3.6
Copy link
Member

Choose a reason for hiding this comment

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

We still support 3.4

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.

@@ -6,9 +6,10 @@ Pyomo CHANGELOG
Current Development
-------------------------------------------------------------------------------

- Resolved Python 3.4 build errors on Appveyor.
- Resolved Python 3.4 build errors on Appveyor. (#438)
Copy link
Member

Choose a reason for hiding this comment

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

This change s a bleed over from #438. At a minimum, we need to resync with master.

Copy link
Member Author

Choose a reason for hiding this comment

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

I already synced with master. This documentation was omitted in that PR. I figured it was a waste of time to have a PR for 7 characters.

.. bash::
conda install -c conda-forge pyomo

Pyomo also has conditional dependencies on a variety of third-party Python packages. These can also be installed with conda:
Copy link
Member

Choose a reason for hiding this comment

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

This document should list the third-party packages, as that is relevant to all distributions /package managers (i.e., pip)

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps it should. But I'm pretty sure that no one will maintain this list here.

Using CONDA
~~~~~~~~~~~

We recommend installation with *conda*, which is included with the Anaconda
Copy link
Member

Choose a reason for hiding this comment

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

I do not recommend conda. I am still confused as to why we don't list pip as the standard install route for pyomo and list conda as a convenience for Anaconda users. Recommending miniconda for non-conda platforms results in a new Python on their system, which will cause problems with IDEs, PATHs, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do recommend conda, because pyomo.extras robustly installs third-party packages. We tried using a similar package for PyPI and users had problem with package installations.

You don't need to install miniconda to use conda.


However, *pip* does not support the robust installation of conditional
dependencies (i.e. the third-party Python packages or the solvers
that Pyomo may use).
Copy link
Member

Choose a reason for hiding this comment

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

I disagree with this statement. pip is the generally recommended installation method for Python packages.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a true statement, regardless whether pip is generally recommended. But I'll delete it.

Optimization solvers are not installed with Pyomo, but some open source optimization solvers can be installed with conda as well:

.. bash::
conda install -c conda-forge pyomo.solvers
Copy link
Member

Choose a reason for hiding this comment

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

This should be conda install -c conda-forge ipopt coin-cbc glpk

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

* CPython: 2.7, 3.4, 3.5, 3.6


Using CONDA
Copy link

Choose a reason for hiding this comment

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

"Conda" not "CONDA"

@jsiirola jsiirola merged commit 2719b5a into master Apr 26, 2018
@jsiirola jsiirola deleted the issue_295 branch April 26, 2018 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing_and_ci Any issues related to testing and/or continuous integration (e.g., github, travis, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants