pgmpy icon indicating copy to clipboard operation
pgmpy copied to clipboard

Add Flake8 Linting to CI Workflow for Mentor-Friendly Code Quality

Open nitishmalang opened this issue 6 months ago • 5 comments

Issue Description

The pgmpy CI workflow (.github/workflows/ci.yml) uses Black for formatting but lacks linting to catch PEP8 style issues like unused imports or complex code. This issue proposes adding a Flake8 linting step to ensure code quality, supporting pgmpy’s PEP8 guidelines (120-character line limit) and making it easier for mentors to guide mentees on contributions like #1054 (BIFReader state name preservation).

Proposed Changes

  • Add a Flake8 step to .github/workflows/ci.yml to run flake8 pgmpy --config=.flake8.
  • Create a .flake8 configuration file to enforce a 120-character line limit and exclude docs and tests.
  • Update Contributing.md to document the Flake8 check.

Benefits

  • Mentors: Clear linting output simplifies guiding mentees on code quality, addressing mentorship challenges like task clarity and feedback.
  • Mentees: Beginner-friendly YAML task with CI feedback, aligning with pgmpy’s mentorship goals.
  • Project: Enhances code quality, supporting recent testing and documentation updates.

Mentor-Friendly Aspects

  • Small task ideal for code review or pair programming.
  • Teaches CI, linting, and PEP8 concepts.
  • Linting reports provide objective feedback for mentors.

Relevance to #1054

Ensures BIFReader code changes comply with PEP8, streamlining PR reviews for #1054.

Extensibility

  • Fix linting errors in files like pgmpy/readwrite/BIF.py.
  • Add pylint or CodeClimate for deeper analysis.
  • Document linting in docs/ci.md.

Labels: Good First Issue, CI/CD, Mentorship, DevOps

nitishmalang avatar May 09 '25 05:05 nitishmalang

@nitishmalang Thanks for raising this issue. I agree it would be good to have a linting tool integrated. So, please feel free to go ahead with this. I am assuming this would result in quite a substantial change in the current codebase to pass the linting checks? I think it would also be nice to add flake8 to .pre-commit-config.yaml.

ankurankan avatar May 09 '25 09:05 ankurankan

Hi @ankurankan! Since this issue's been dormant for some time, would it be okay if I take it or is it being worked on?

KapilSareen avatar May 25 '25 17:05 KapilSareen

Hi @ankurankan! Since this issue's been dormant for some time, would it be okay if I take it or is it being worked on?

Hi @KapilSareen I am working on this I'm inactive due to finals I'll be back after two days You may work on other issue Thank-You

nitishmalang avatar May 25 '25 17:05 nitishmalang

Hi @ankurankan Sir. I have worked on an issue similar to this before, in which I was removing the errors of maximum line length should be less than 120, and it was merged. There are some things remaining as mentioned in this issue. If you don't mind can I work on this issue. Please assign it to me! I would definitely love to work on this issue.

DarshanCode2005 avatar Jun 05 '25 18:06 DarshanCode2005

Hi @ankurankan Sir. I have worked on an issue similar to this before, in which I was removing the errors of maximum line length should be less than 120, and it was merged. There are some things remaining as mentioned in this issue. If you don't mind can I work on this issue. Please assign it to me! I would definitely love to work on this issue.

Hi @ankurankan assign this issue to @DarshanCode2005 or @KapilSareen because I am finding difficulty in solving this while these people can do it better while i will raise and contribute to another issue Thank-You

nitishmalang avatar Jun 05 '25 22:06 nitishmalang