8000 LGTM alerts . · Issue #19077 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

LGTM alerts . #19077

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.

Sign up for GitHub

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

Jump to bottom
Closed
charris opened this issue May 23, 2021 · 33 comments · Fixed by #19090 or #19102
Closed

LGTM alerts . #19077

charris opened this issue May 23, 2021 · 33 comments · Fixed by #19090 or #19102

Comments

@charris
Copy link
Member
charris commented May 23, 2021

I notice that there are still a fair number LGTM alerts, many of which look pretty simple. I wonder if they would be a good source of issues for the next NumPy sprint?
@ajayd-san
Copy link
Contributor

hey, can i have this issue?
I'm quite new to this so I might need some guidance

@ganesh-k13
Copy link
Member

Posting for reference the LGTM alerts: https://lgtm.com/projects/g/numpy/numpy/alerts/?mode=list

I can take a look at the errors in arithmetic.h, which were added during our SIMD division change.

@ajayd-san
Copy link
Contributor

@ganesh-k13 so i just need to fix the errors/warnings shown in the lgtm page ?

@mattip
Copy link
Member
mattip commented May 24, 2021

I'm quite new to this so I might need some guidance

Welcome.

Please read the developer's workflow guide, especially around commit messages.

Some of those errors/warnings will be difficult to fix since LGTM's ideas may not agree with ours. Try to group the fixes in some logical way: for instance, all the unused imports in one PR and in another PR all the integer overflow warnings.

@ajayd-san
Copy link
Contributor

@mattip thanks.

I've removed all of the unused imports only to find out it breaks the code fsr(hoping you'll tell why), but reading the exceptions and restoring the removed imports solved the issue.
I've restored the following imports -

I can now build and run tests. The test results are same after removing unused imports as linked below.

Before fixing imports -
before_fixing_unused_imports

After fixing imports -
after_fixing_unused_imports

Do you want me to go ahead and do a pr ?

@ajayd-san
Copy link
Contributor
ajayd-san commented May 25, 2021 •
edited
Loading

@mattip do you want me to go ahead and do a pr man? or is there I should be doing first ?

Thanks

@mattip
Copy link
Member
mattip commented May 25, 2021

A cleanup PR would be welcome. Please be sure it only changes the lines you intend to change: sometimes coding tools can "clean up" formatting which makes PRs harder to review.

@ajayd-san ajayd-san mentioned this issue May 25, 2021
Merged
@charris charris reopened this May 25, 2021
@ajayd-san ajayd-san mentioned this issue May 26, 2021
Merged
@ajayd-san
Copy link
Contributor

@charris I've raised a new PR for unused variables. Can anyone check and give a suitable review?

@mattip
Copy link
Member
mattip commented May 26, 2021

Usually review requires a few days. If after a week no-one has reviewed, feel free to ping (without names) in the PR with a "please review" comment.

@ajayd-san
Copy link
Contributor

thanks, noted.

@ganesh-k13 ganesh-k13 mentioned this issue May 27, 2021
Closed
wandrewjam added a commit to wandrewjam/numpy that referenced this issue Jul 1, 2021
@wandrewjam
MAINT: fix overly broad exception handling listed in LGTM
7949ba5 
Relates to ticket numpy#19077
@wandrewjam wandrewjam mentioned this issue Jul 1, 2021
Merged
@charris charris reopened this Jul 9, 2021
@charris
Copy link
Member Author
charris commented Jul 9, 2021

Keeping this open for a while.

@ManjunathBharadwaj01
Copy link

Hello, can I contribute to this particular issue?
I'm new , and want to learn the GitHub ways, can somebody help me learn?

@kurchi1205
Copy link

https://lgtm.com/projects/g/numpy/numpy/alerts/?mode=list
I want to make my first contribution by fixing version.py .
Can I work on it?

@mattip
Copy link
Member
mattip commented Aug 20, 2021

I want to make my first contribution by fixing version.py .

What would you fix? I am not seeing any LGTM alerts for numpy/version.py (last file in the link). Or perhaps you meant the single miniscule warning in numpy/_version.py ?

@kurchi1205
Copy link

It is listed under error

@eric-wieser
Copy link
Member
eric-wieser commented Aug 20, 2021 •
edited
Loading

That file is a generated file. The correct fix will be to tell lgtm (and probably gitattributes) to ignore that file. You could fix it upstream in versioneer too if you wanted.

@MansiSMore
Copy link

Hello, I am new to open source contribution, Can I fix the "Suspicious unused loop iteration variable" issue from [https://lgtm.com/projects/g/numpy/numpy/alerts/?mode=list] ???

@mattip
Copy link
Member
mattip commented Aug 22, 2021

We do not assign work. Anyone can submit a PR. Please mark that as related to #19077 so others will see that you are working on it.

@dhivyasreedhar
Copy link

Can I work on this? I'm new so can someone guide me?

@Sreejan100
Copy link

@charris can I contribute in this? I am a starter in open source contributions and therefore may require guidance

@Stark-developer01
Copy link

Is the issue fixed? Otherwise I would take up the issue.

@priorigratia
Copy link

I would want to take this issue up if it is still open

@charris
Copy link
Member Author
charris commented Aug 29, 2021

@priorigratia Go ahead, but keep in mind that some of the remaining issues are subtle and harder than they look.

@iamsatya8
Copy link

Let Me Do this issue

@mattip
Copy link
Member
mattip commented Aug 29, 2021

We do not assign work. Anyone can submit a PR. Please mark that as related to #19077 so others will see that you are working on it.

@sistaseetaram
Copy link
Contributor

Hi, I would like to work on the following issue. regexp miss-match. line 227. But I'm confused about what we're trying to match? Can you please provide test cases so that I can work on it. This is my first project so kindly help me.

@eric-wieser
Copy link
Member

Hi, I would like to work on the following issue. regexp miss-match. line 227. But I'm confused about what we're trying to match? Can you please provide test cases so that I can work on it. This is my first project so kindly help me.

This code path is exercised by https://github.com/numpy/numpy/blob/main/numpy/linalg/lapack_lite/README.rst

@charris charris mentioned this issue Sep 2, 2021
Merged
@sistaseetaram
Copy link
Contributor

I've gone through it.. and now I'm a bit confused. will you please guide me through it? There are two types of subroutines in f2c_*.c, extern subroutines and local subroutines, and I am a bit confused whether we're trying to match both or only local subroutines.
Given pattern string:
r'/\* Subroutine \*/^\s*(?:(?:inline|static)\s+){0,2}(?!else|typedef|return)\w+\s+\*?\s*(\w+)\s*\([^0]+\)\s*;?'
As this above regex has multiple errors.

  1. it matches no string as ^ is included in between
  2. it matches only single line subroutines as stringIO splits at \n and subroutines can be multi-line.
    I think it should be changed to
    r'/\* Subroutine \*/\s*(?:(?:inline|static)\s+){0,2}(?!else|typedef|return)\w+\s+\*?\s*(\w+)\s*\([^0]+|[\n\s]*\)\s*;?'
    suggest changes if wrong. I just started to work on open source so sorry for long messages.

@MaciejWas MaciejWas mentioned this issue Sep 6, 2021
Closed
@acse-srm3018
A3E2 Copy link

Hi everyone, I'd like to contribute to this issue. I am a beginner in contributing to open source projects. Could someone help and guide me on how to start a contribution?

@ovynnej
Copy link
ovynnej commented Oct 1, 2021 •
edited
Loading

@acse-srm3018 If you're still interested in working on this issue (or others), here's a summary for first-time contributors to numpy!

@Jbyford89
Copy link

Hello I have read the first-time contributors docs and would like to contribute. What exactly do I do with the issues, I am having a little trouble following.

@sistaseetaram
Copy link
Contributor

Posting for reference the LGTM alerts: https://lgtm.com/projects/g/numpy/numpy/alerts/?mode=list

The above link has LGTM errors and alerts, this issue is all about fixing them. Visit the link, you will understand it.

@charris
Copy link
Member Author
charris commented Nov 7, 2021

I'm going to close this as we are disabling LGTM. Continuing work to fix the already found problems is of course welcome.

@charris charris closed this as completed Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
0