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

Lint the whole codebase and add coverage test. #317

Merged
merged 29 commits into from
May 28, 2021

Conversation

lsetiawan
Copy link
Member

@lsetiawan lsetiawan commented Apr 30, 2021

This PR cleans up code formatting and styling to meet pep8 coding conventions. Additionally, code coverage tests are added. Also, the setup configurations are now mostly in setup.cfg rather than setup.py.

Note: This PR is not ready for review or merging as of 4/30/2021.
Update: This PR is now ready for review and the lint tests are now passing.

@lsetiawan lsetiawan modified the milestones: 0.5.0 release, 0.5.1 release May 3, 2021
lsetiawan and others added 8 commits May 24, 2021 13:06
@lsetiawan lsetiawan changed the base branch from master to dev May 24, 2021 22:03
@lsetiawan lsetiawan requested a review from leewujung May 24, 2021 22:42
@lsetiawan lsetiawan changed the title Clean up code styling and formatting Lint the whole codebase and add coverage test. May 24, 2021
@codecov-commenter
Copy link

codecov-commenter commented May 25, 2021

Codecov Report

❗ No coverage uploaded for pull request base (dev@0109551). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head be9587c differs from pull request most recent head 6a06fa1. Consider uploading reports for the commit 6a06fa1 to get more accurate results
Impacted file tree graph

@@          Coverage Diff          @@
##             dev    #317   +/-   ##
=====================================
  Coverage       ?   1.69%           
=====================================
  Files          ?      35           
  Lines          ?    3008           
  Branches       ?       0           
=====================================
  Hits           ?      51           
  Misses         ?    2957           
  Partials       ?       0           
Flag Coverage Δ
unittests 1.69% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0109551...6a06fa1. Read the comment docs.

@lsetiawan
Copy link
Member Author

@leewujung This is all ready to go with all the latest changes for QC and Imran's ADCP Time Encoding issue fixes, linted. 😄

@lsetiawan
Copy link
Member Author

lsetiawan commented May 25, 2021

Let me know when this is good to merge. I'll need to merge it so that I can clean up the commit message a little bit, since it's currently very messy. Thanks!

Comment on lines 143 to 145
a = self.cal_params[
"DS"
] # scaling factor (slope) in Fig.G-1, units Volts/dB], see p.84
Copy link
Member

Choose a reason for hiding this comment

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

Not very readable, would be better to move the comment on top of the line and retain the code in one line

Copy link
Member

@leewujung leewujung left a comment

Choose a reason for hiding this comment

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

@lsetiawan Thanks for the linting effort!
I went through all changes quickly with a spirit of spotting anomalies, and found a few things that I commented on. Let me know what you think. Otherwise I think this is ready to be merged.

@lsetiawan lsetiawan merged commit f78d16d into OSOceanAcoustics:dev May 28, 2021
@lsetiawan lsetiawan deleted the formatting branch May 28, 2021 19:48
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.

None yet

4 participants