Skip to content

Reducing number of pytest warnings#1067

Merged
erikvansebille merged 4 commits into
masterfrom
reducing_pytest_warnings
Jul 26, 2021
Merged

Reducing number of pytest warnings#1067
erikvansebille merged 4 commits into
masterfrom
reducing_pytest_warnings

Conversation

@erikvansebille
Copy link
Copy Markdown
Member

This PR fixes some of the warnings that pop up when running pytest. Most of these are beyond our control (warning in other packages), but there were also a few in Parcels itself that this PR fixes

@erikvansebille erikvansebille requested a review from CKehl July 22, 2021 14:08
Copy link
Copy Markdown
Contributor

@CKehl CKehl left a comment

Choose a reason for hiding this comment

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

the changes in base particle set and test_kernel_language look fine to me, so they are good. The change in example_peninsula I don't really understand why it was necessary.

Code-wise, it looks fine.

Comment on lines +50 to +51
La = np.linspace(1e3, domainsizeX, xdim, dtype=np.float32)
Wa = np.linspace(1e3, domainsizeY, ydim, dtype=np.float32)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this change I don't really understand - how does the shrinking for the field size gets rid of a pytest warning ? Which pytest warning was that ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, good that you ask. It was a division-by-zero warning.

example_peninsula.py:59: RuntimeWarning: invalid value encountered in true_divide
P = (u0*R**2*y/((x-x0)**2+y**2)-u0*y) / 1e3

This is now avoided by making sure no zeroes in x and y

@erikvansebille erikvansebille merged commit 50a8f19 into master Jul 26, 2021
@CKehl CKehl deleted the reducing_pytest_warnings branch September 27, 2022 16:19
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