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

Add progress bars #2

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

MatthewKhouzam
Copy link
Collaborator

This should be reviewed after #1

Vlad Arama and others added 4 commits January 29, 2024 15:37
Use Black Code style and pylint checker
Update log_anonymizer.py (linting and cleanup)
Copy link
Owner

@vladarama vladarama left a comment

Choose a reason for hiding this comment

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

I love the progress bars Matthew, very cool. I suggest we use the file_path instead of filename for the count_lines function, check my comments.


with open(input_file_path, 'r') as input_file, open(output_file_path, 'w') as output_file:
for line in input_file:
line_count = count_lines(filename=file_name)
Copy link
Owner

@vladarama vladarama Jan 31, 2024

Choose a reason for hiding this comment

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

Suggested change
line_count = count_lines(filename=file_name)
line_count = count_lines(file_path=input_file_path)

I am running into issues on this line, python cannot find the file.
I think we need to provide the file path instead. This should make it work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a rebase deal. Yes, your patch suggestions are valid! :)

@@ -279,48 +357,81 @@ def randomize_numbers(number, is_ip_address = False):
return random.randint(lower_bound, upper_bound)


def count_lines(filename:str)->int:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
def count_lines(filename:str)->int:
def count_lines(file_path:str)->int:

@@ -279,48 +357,81 @@ def randomize_numbers(number, is_ip_address = False):
return random.randint(lower_bound, upper_bound)


def count_lines(filename:str)->int:
c = 0
with open(filename) as file:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
with open(filename) as file:
with open(file_path) as file:

@MatthewKhouzam
Copy link
Collaborator Author

Trying the "resolve confilicts" button. Maybe it broke everything?

Signed-off-by: MatthewKhouzam <[email protected]>
@MatthewKhouzam
Copy link
Collaborator Author

I think it's done?

@vladarama vladarama self-requested a review February 1, 2024 14:29
Update count_lines
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