Skip to content

Fix lgtm warnings in quadsums.py.#541

Merged
mmcky merged 1 commit intoQuantEcon:masterfrom
duncanhobbs:lgtm-warn-quadsums
May 10, 2020
Merged

Fix lgtm warnings in quadsums.py.#541
mmcky merged 1 commit intoQuantEcon:masterfrom
duncanhobbs:lgtm-warn-quadsums

Conversation

@duncanhobbs
Copy link
Copy Markdown
Contributor

This pull request addresses this lgtm alert.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.001%) to 93.921% when pulling 75d30b6 on duncanhobbs:lgtm-warn-quadsums into b722605 on QuantEcon:master.

@mmcky
Copy link
Copy Markdown
Contributor

mmcky commented May 10, 2020

thanks @duncanhobbs these are interesting suggestions by lgtm. I tend to prefer the np.sqrt reference to know which function I am using in location without looking at the top of the code but if lgtm wants this style I guess I am ok with that :-)

@oyamad do you have a preference here?

Copy link
Copy Markdown
Member

@oyamad oyamad left a comment

Choose a reason for hiding this comment

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

@mmcky I am not sure if I understand which you prefer, but I prefer

import numpy as np
...
... np.sqrt(beta) ...

to

from numpy import sqrt
...
... sqrt(beta) ...

@mmcky
Copy link
Copy Markdown
Contributor

mmcky commented May 10, 2020

Thanks @oyamad I prefer the first style as well.

It appears LGTM is recommending the second.

@mmcky
Copy link
Copy Markdown
Contributor

mmcky commented May 10, 2020

Sorry @duncanhobbs and @oyamad I am being dyslexic here. These changes are great as they are using the preferred style np.sqrt

@mmcky mmcky merged commit 0e56dba into QuantEcon:master May 10, 2020
@duncanhobbs duncanhobbs deleted the lgtm-warn-quadsums branch May 10, 2020 13:50
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.

4 participants