Skip to content
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

Chore/add qoc config #62

Merged
merged 6 commits into from
Feb 6, 2023
Merged

Chore/add qoc config #62

merged 6 commits into from
Feb 6, 2023

Conversation

Batalex
Copy link
Contributor

@Batalex Batalex commented Feb 2, 2023

This PR adds a few quality of code tools to help with long-term maintenance.

  • ruff configuration: The provided configuration enables flake8 & isort rules. Here is the transcript of the run that changed files with those rules.
`ruff .`
src/pynteny/__init__.py:1:1: I001 Import block is un-sorted or un-formatted
src/pynteny/api.py:8:1: I001 Import block is un-sorted or un-formatted
src/pynteny/app/callbacks.py:6:1: I001 Import block is un-sorted or un-formatted
src/pynteny/app/components.py:1:1: I001 Import block is un-sorted or un-formatted
src/pynteny/app/components.py:76:13: F841 Local variable `output_log` is assigned to but never used
src/pynteny/app/filemanager.py:6:1: I001 Import block is un-sorted or un-formatted
src/pynteny/app/filemanager.py:59:5: E722 Do not use bare `except`
src/pynteny/app/filemanager.py:74:5: E722 Do not use bare `except`
src/pynteny/app/helpers.py:4:1: I001 Import block is un-sorted or un-formatted
src/pynteny/app/main_page.py:1:1: I001 Import block is un-sorted or un-formatted
src/pynteny/cli.py:4:1: I001 Import block is un-sorted or un-formatted
src/pynteny/cli.py:125:9: F841 Local variable `args` is assigned to but never used
src/pynteny/cli.py:521:9: F841 Local variable `required` is assigned to but never used
src/pynteny/cli.py:539:9: F841 Local variable `required` is assigned to but never used
src/pynteny/cli.py:546:5: F841 Local variable `pynteny` is assigned to but never used
src/pynteny/filter.py:8:1: I001 Import block is un-sorted or un-formatted
src/pynteny/hmm.py:8:1: I001 Import block is un-sorted or un-formatted
src/pynteny/parsers/labelparser.py:8:1: I001 Import block is un-sorted or un-formatted
src/pynteny/parsers/syntenyparser.py:8:1: I001 Import block is un-sorted or un-formatted
src/pynteny/parsers/syntenyparser.py:13:8: F401 `pandas` imported but unused
src/pynteny/preprocessing.py:12:1: I001 Import block is un-sorted or un-formatted
src/pynteny/preprocessing.py:144:21: F541 f-string without any placeholders
src/pynteny/subcommands.py:8:1: I001 Import block is un-sorted or un-formatted
src/pynteny/subcommands.py:220:5: F841 Local variable `module_dir` is assigned to but never used
src/pynteny/subcommands.py:248:25: F841 Local variable `e` is assigned to but never used
src/pynteny/utils.py:8:1: I001 Import block is un-sorted or un-formatted
src/pynteny/wrappers.py:8:1: I001 Import block is un-sorted or un-formatted
tests/test_database_build.py:8:1: I001 Import block is un-sorted or un-formatted
tests/test_database_build.py:33:13: F841 Local variable `labelled_database` is assigned to but never used
tests/test_hmm.py:7:1: I001 Import block is un-sorted or un-formatted
tests/test_parser.py:8:1: I001 Import block is un-sorted or un-formatted
tests/test_preprocessing.py:8:1: I001 Import block is un-sorted or un-formatted
Found 32 errors.
30 potentially fixable with the --fix option.

The two "bare except" errors are silenced using Exception. This is only a bit better than bare except (I will not bore you with the details, but bare except catches stuff like SIGINT signal with Ctrl + C, whereas Exception is the superclass of all user-defined exceptions)

  • pre-commit configuration. Pre-commit allows "hooks" to run before committing files. I propose the two following hooks to get started: black & ruff.
  • To be done: CI checks

@codecov
Copy link

codecov bot commented Feb 2, 2023

Codecov Report

Base: 97.50% // Head: 97.50% // No change to project coverage 👍

Coverage data is based on head (b3a5f9e) compared to base (8b5f87b).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #62   +/-   ##
=======================================
  Coverage   97.50%   97.50%           
=======================================
  Files           5        5           
  Lines         160      160           
=======================================
  Hits          156      156           
  Misses          4        4           
Impacted Files Coverage Δ
tests/test_parser.py 96.77% <ø> (ø)
tests/test_database_build.py 100.00% <100.00%> (ø)
tests/test_hmm.py 97.22% <100.00%> (ø)
tests/test_preprocessing.py 98.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@Robaina Robaina left a comment

Choose a reason for hiding this comment

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

Great! Thanks for the PR.

I see ruff got busy with all those changes, it is a cool tool. There is one case I have to double-check: I see that ruff removes unused variables, which makes sense. However, I have to double-check the case of pynteny = Pynteny(...), I think it was there to prevent unwanted text from being prompted.

src/pynteny/cli.py Show resolved Hide resolved
src/pynteny/subcommands.py Show resolved Hide resolved
src/pynteny/cli.py Show resolved Hide resolved
@Batalex
Copy link
Contributor Author

Batalex commented Feb 3, 2023

@Robaina This PR adds a bit of noise across the files, with what could be perceived as changes without much value.
The benefits will be more apparent in future PRs. Let's take black for example; one of its goals is to minimize the difference between changes (as in git line changes), which means that once you start using it on a per-commit basis, the PRs on GitHub will be shorter, hence easier to read and review.

The same applies to the import sorting, now that they are sorted, it will be easier to spot the changes in the organized sections:

  • future
  • standard lib
  • third party
  • first party: pynteny itself

@Batalex Batalex marked this pull request as ready for review February 3, 2023 17:34
@Robaina Robaina closed this Feb 5, 2023
@Robaina Robaina reopened this Feb 5, 2023
@Robaina
Copy link
Owner

Robaina commented Feb 5, 2023

@Robaina This PR adds a bit of noise across the files, with what could be perceived as changes without much value. The benefits will be more apparent in future PRs. Let's take black for example; one of its goals is to minimize the difference between changes (as in git line changes), which means that once you start using it on a per-commit basis, the PRs on GitHub will be shorter, hence easier to read and review.

The same applies to the import sorting, now that they are sorted, it will be easier to spot the changes in the organized sections:

  • future
  • standard lib
  • third party
  • first party: pynteny itself

Right, I understand and agree on the benefits of using these tools. Nice tools! As I wrote before, I'm thinking of integrating the linting within the test.yml as it basically is there to do CI stuff.

Copy link
Owner

@Robaina Robaina left a comment

Choose a reason for hiding this comment

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

Approving merge

.github/workflows/ci.yml Show resolved Hide resolved
src/pynteny/cli.py Show resolved Hide resolved
src/pynteny/cli.py Show resolved Hide resolved
@Robaina Robaina merged commit 4079594 into Robaina:main Feb 6, 2023
@Batalex Batalex deleted the chore/add-qoc-config branch February 9, 2023 08:49
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