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

Drop support for IPython < 3.0. #237

Merged
merged 1 commit into from
Jun 16, 2015
Merged

Conversation

moorepants
Copy link
Member

Fixes #235.

Also add PR links to some items in the release notes.

I've tested each of the import errors and warnings with no IPython, IPython < 3.0, and IPython 3.0+. Everything works for me locally.

  • There are no merge conflicts.
  • If there is a related issue, a reference to that issue is in the
    commit message.
  • Unit tests have been added for the bug. (Please reference the issue #
    in the unit test.)
  • The tests pass both locally (run nosetests) and on Travis CI.
  • The code follows PEP8 guidelines. (use a linter, e.g.
    pylint, to check your code)
  • The bug fix is documented in the Release
    Notes
    .
  • The code is backwards compatible. (All public methods/classes must
    follow deprecation cycles.)
  • All reviewer comments have been addressed.

@moorepants moorepants added this to the 0.3.0 Release milestone Jun 16, 2015
@moorepants
Copy link
Member Author

@pydy/pydy-developers This needs a review for the 0.3.0 release.

@oliverlee oliverlee changed the title Drop support for IPython <= 3.0. Drop support for IPython < 3.0. Jun 16, 2015
msg = ('PyDy only supports IPython >= 3.0.0. You have '
'IPython {} installed. IPython related functionalities will '
'not be available')
with warnings.catch_warnings():
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use catch_warnings(). It should be used if you expect someone else's code to throw warnings.

Also why do you need to specify once? This module shouldn't be imported multiple times.

Copy link
Member Author

Choose a reason for hiding this comment

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

By default Python does not show warnings and I only want these warnings to show once per python session. So if you happen to import this multiple times in single session it will only issue the import warning once.

catch_warnings is a nice way to set the filters on a per warning level. I'd also be fine with setting the warning filter to once globaly for PyDy. If you have a suggestion of a better pattern for per warning filter control, I can change it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just use the warning filter with once. The way you have it written was causing the Deprecation Warning to be displayed every single time the method was called when using Python 3.

Copy link
Member Author

Choose a reason for hiding this comment

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

So what is the difference in Python 3 about this. I don't understand.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason I used this way of setting the filter was because if you set the warnings filter globally in a module, you'll turn on all warnings for that python session, including, I believe, all the warnings that are normally hidden in any external module you import. Maybe this was a problem when we were testing against NumPy 1.6 and SciPy 0.9.

Copy link
Member Author

Choose a reason for hiding this comment

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

You suggestion requires creating sublcassed warnings. I see why you did that in your other PR. I guess I never did that because it seems silly to create a subclass that doesn't have anything overridden. I'll create a PyDyImportWarning that subclasses ImportWarning.

Copy link
Contributor

Choose a reason for hiding this comment

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

In Python 3, the filter is only applied in the scope of the with warnings.catch_warnings() block.

Here's a short example:

import warnings

def func():
    with warnings.catch_warnings():
        warnings.filterwarnings('once'):
        warnings.warn('asd', ImportWarning)
        warnings.warn('asd', ImportWarning)

for i in range(2):
    func()

If you run with Python 2 and Python 3, it prints the warning a different number of times.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'll change to what I said above.

@oliverlee
Copy link
Contributor

Please change the commit title to:
Drop support for IPython < 3.0

Fixes pydy#235.

- Adds in a PyDyImportWarning.
- Also add PR links to some items in the release notes.
- Fixes an old error in MANIFEST.in.
@moorepants
Copy link
Member Author

Ok, things are fixed.

- Added support Python 3.3 and 3.4. [PR `#38`_]
- Bumped up the minimum dependencies for NumPy, SciPy, and Cython.
- Dropped support for IPython < 3.0. [PR `#237`_]
- Added support Python 3.3 and 3.4. [PR `#229`_]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this here? I thought it was already merged 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 rebased and then changed it here to point to the PR instead of the original issue to follow the same pattern as the others.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool thanks

oliverlee added a commit that referenced this pull request Jun 16, 2015
Drop support for IPython < 3.0.
@oliverlee oliverlee merged commit 4f342a0 into pydy:master Jun 16, 2015
@moorepants moorepants deleted the drop-ipython2 branch June 18, 2015 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop support for IPython 2
2 participants