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 #2327

Merged

Conversation

ericpre
Copy link
Member

@ericpre ericpre commented Feb 29, 2020

Progress of the PR

  • Specify iterpath='serpentine' in the test suite to avoid deprecation warning,
  • add numpy module to the Voigt components to avoid deprecation warning about scipy.real
  • ready for review.

…y deprecation warning: "DeprecationWarning: scipy.real is deprecated and will be removed in SciPy 2.0.0, use numpy.real instead".
@@ -250,7 +250,8 @@ def setup_method(self, method):
def test_lines_intensity(self):
s = self.s
m = s.create_model()
m.multifit() # m.fit() is just too inaccurate
# HyperSpy 2.0: remove setting iterpath='serpentine'
m.multifit(iterpath='serpentine') # m.fit() is just too inaccurate
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's an old comment, but can we quantify what we mean by # m.fit() is just too inaccurate

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, this is a good point, that sounds odd to me and it would be good to do something about it!

Copy link
Member Author

Choose a reason for hiding this comment

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

It look to me that this statement is not correct now and we can simply removed it. I added a test to confirm this but I think we could also remove this test comparing fit and multifit.
This comment was commited 4 years ago (5b3b92f) and the commit message doesn't say what could the reason for this comment, so I don't see any reason to keep it, unless it is possible to demonstrate that it doesn't work.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the test is cheap to run, I'd vote to keep it.

@francisco-dlp francisco-dlp merged commit 7399e49 into hyperspy:RELEASE_next_minor Apr 3, 2020
@ericpre ericpre deleted the fit_deprecation_warning branch April 3, 2020 15:44
@ericpre ericpre added this to the v1.6 milestone Apr 3, 2020
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.

3 participants