Skip to content

LGTM has 6 errors #1275

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

Closed
mikofski opened this issue Aug 9, 2021 · 5 comments · Fixed by #1559
Closed

LGTM has 6 errors #1275

mikofski opened this issue Aug 9, 2021 · 5 comments · Fixed by #1559

Comments

@mikofski
Copy link
Member

mikofski commented Aug 9, 2021

Describe the bug
LGTM shows 6 errors, at least one seems easy to fix there is a self alignment that seems redundant

To Reproduce
https://lgtm.com/projects/g/pvlib/pvlib-python/alerts/?mode=list&severity=error

Expected behavior
No errors?

Screenshots
https://lgtm.com/projects/g/pvlib/pvlib-python/alerts/?mode=list&severity=error

Versions:

  • pvlib.__version__: 0.9ish
  • pandas.__version__: ?
  • python: 3

Additional context
https://lgtm.com/projects/g/pvlib/pvlib-python/alerts/?mode=list&severity=error

@mikofski mikofski added the bug label Aug 9, 2021
@wholmgren
Copy link
Member

Certainly should fix that self assignment. Maybe silence the others?

@mikofski
Copy link
Member Author

mikofski commented Aug 9, 2021

I found a Python "LGTM alert suppression" example here: https://lgtm.com/help/lgtm/alert-suppression#python

Altho I think we should try to learn a bit about why LGTM raises these alerts before deciding to silence them. Could be something useful?

@wholmgren
Copy link
Member

I've clicked the Hide button on a handful of similar alerts in the past. I didn't know about the comment option until now.

Screen Shot 2021-08-09 at 9 27 53 AM

I tried looking into it a few years ago and didn't figure it out. Maybe someone smarter than me can figure it out.

hf-kklein added a commit to hf-kklein/pvlib-python that referenced this issue May 21, 2022
This addresses 1 of the problems from pvlib#1275; The easiest tbh
kandersolar pushed a commit that referenced this issue Jun 13, 2022
This addresses 1 of the problems from #1275; The easiest tbh
@chrisorner
Copy link
Contributor

I've clicked the Hide button on a handful of similar alerts in the past. I didn't know about the comment option until now.

Screen Shot 2021-08-09 at 9 27 53 AM

I tried looking into it a few years ago and didn't figure it out. Maybe someone smarter than me can figure it out.

This is a bug in LGTM. It raises this error because of the user function test in test_modelchain.py:
image

Where constant_aoi_loss is a Python partial function which is the known bug in LGTM. So this error can be ignored. I am going to take a look at the others as well.

@chrisorner
Copy link
Contributor

I am pretty sure that the alerts under non-callable can be ignored as well:
image

I described the issue here because I think it's also a bug in LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants