-
Notifications
You must be signed in to change notification settings - Fork 0
Lab 3 #3
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
base: main
Are you sure you want to change the base?
Conversation
Maybe I don`t do it in time
…trange and the program seems to freeze)
lab 3/main.py
Outdated
logging.debug('Debug message') | ||
logging.info('Info message') | ||
logging.warning('Warning message') | ||
logging.error('Error message') | ||
logging.critical('Critical message') | ||
logging.info('1 step') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary code
lab 3/main.py
Outdated
self.file_out_name = des_fold_path + "\\" + f_name + '.csv' | ||
self.data = self.download_data() | ||
print(self.file_out_name) | ||
logging.info('Made class : set Destination folder and Filename ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use standard logger. make a custom one
lab 3/main.py
Outdated
def __init__(self, des_fold_path, f_name='output.csv'): | ||
self.data_file = 'lab3.csv' | ||
self.des_fold_path = des_fold_path | ||
self.previous_output = f_name + '.csv' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f-string
lab 3/main.py
Outdated
self.data_file = 'lab3.csv' | ||
self.des_fold_path = des_fold_path | ||
self.previous_output = f_name + '.csv' | ||
self.file_out_name = des_fold_path + "\\" + f_name + '.csv' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os.path.join
don't use double-quotes
lab 3/main.py
Outdated
self.previous_output = f_name + '.csv' | ||
self.file_out_name = des_fold_path + "\\" + f_name + '.csv' | ||
self.data = self.download_data() | ||
print(self.file_out_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need prints in the code
lab 3/main.py
Outdated
def download_data(self): | ||
logging.info('1. Downloading data') | ||
respons = requests.get('https://randomuser.me/api/?results=5000&format=csv') | ||
open(self.data_file, mode='w', encoding='utf-8').write(respons.text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you didn't close write stream!!!
use context manager here
lab 3/main.py
Outdated
data_dicts = [] | ||
with open(self.data_file, 'r', encoding='utf-8') as file: | ||
csv_reader = csv.DictReader(file) | ||
for row in csv_reader: | ||
data_dicts.append(row) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data_dicts = [] | |
with open(self.data_file, 'r', encoding='utf-8') as file: | |
csv_reader = csv.DictReader(file) | |
for row in csv_reader: | |
data_dicts.append(row) | |
with open(self.data_file, 'r', encoding='utf-8') as file: | |
data_dicts = list(csv.DictReader(file)) |
lab 3/main.py
Outdated
def add_to_output_file(self, save_output): | ||
with open(self.previous_output, 'w', encoding='utf-8') as csv_output: | ||
writer = csv.DictWriter(csv_output, fieldnames=save_output[0].keys()) | ||
writer.writeheader() | ||
writer.writerows(save_output) | ||
logging.info('add_to_output_file correct') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the method looks nice
logging.basicConfig( | ||
filename="../file.log", | ||
level=logging.INFO, | ||
format="%(asctime)s:%(levelname)s:%(name)s:%(message)s", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use basic logger.
make a custom one
import requests | ||
|
||
logging.basicConfig( | ||
filename="../file.log", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use double quotes
lab3/src/data_humans.py
Outdated
|
||
class DataHumans: | ||
def __init__(self, des_fold_path, f_name="output.csv"): | ||
self.data_file = "../lab3.csv" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a constant. make it as a class atribute
lab3/src/data_humans.py
Outdated
def __init__(self, des_fold_path, f_name="output.csv"): | ||
self.data_file = "../lab3.csv" | ||
self.des_fold_path = des_fold_path | ||
self.previous_output = f_name + ".csv" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f-string
lab3/src/data_humans.py
Outdated
self.data_file = "../lab3.csv" | ||
self.des_fold_path = des_fold_path | ||
self.previous_output = f_name + ".csv" | ||
self.file_out_name = os.path.join(des_fold_path, f"{f_name}.csv") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't you use self.previous_output here
lab3/src/data_humans.py
Outdated
|
||
def download_data(self): | ||
logging.info("1. Downloading data") | ||
respons = requests.get("https://randomuser.me/api/?results=5000&format=csv") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make this url as a constant
lab3/src/data_humans.py
Outdated
def download_data(self): | ||
logging.info("1. Downloading data") | ||
respons = requests.get("https://randomuser.me/api/?results=5000&format=csv") | ||
with open(self.data_file, mode="w", encoding="utf-8") as file: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make utf-8 another constant
lab3/src/data_humans.py
Outdated
csv_reader = csv.DictReader(file) | ||
return [row for row in csv_reader] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
csv_reader = csv.DictReader(file) | |
return [row for row in csv_reader] | |
return list(csv.DictReader(file)) |
lab3/src/data_humans.py
Outdated
with open(self.data_file, "r", encoding="utf-8") as file: | ||
data_dicts = list(csv.DictReader(file)) | ||
logging.info(f"downloading correct ,len of file{len(data_dicts)}") | ||
return data_dicts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't you use read_data_from_file method
lab3/src/data_humans.py
Outdated
csv_reader = csv.DictReader(file) | ||
return [row for row in csv_reader] | ||
|
||
def numbering_rows(self, idx, value): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name of functions and methods should be verbs/actions not nouns
def file_replace(self): | ||
logging.info("7. Move initial file to the destination folder") | ||
if not os.path.exists(self.file_out_name): | ||
os.makedirs(self.des_fold_path, mode=777) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
google exist_ok=true parameter
return user | ||
|
||
def the_most_old(self, user): # использовать max lst | ||
return max([int(f["dob.age"]) for f in user]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you call variable f
return collections.Counter(lst_for_count).most_common(1)[0][0] | ||
|
||
def make_name_of_file(self, basic_path, user_data): | ||
return f'{basic_path}\\max_age_{str(self.the_most_old(user_data))}_avg_registered_{str(self.average_year_of_reg(user_data))}_ popular_id_{str(self.popular_genres(user_data))}_user_data_{str(user_data[0]["global_index"])}.csv' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please check if you really need conversion to str
lab3/src/data_humans.py
Outdated
logging.info("9. 10 . make dir for decade") | ||
for i in user_data: | ||
for c in user_data[i]: | ||
basic_path = f"{self.des_fold_path}\\{i}\\{c}" | ||
os.makedirs(basic_path) # f-string \\ make variable//make func | ||
with open( | ||
self.make_name_of_file(basic_path, user_data[i][c]), | ||
"w", | ||
encoding="utf-8", | ||
) as csv_output: | ||
writer = csv.DictWriter( | ||
csv_output, fieldnames=user_data[i][c][0].keys() | ||
) | ||
writer.writeheader() | ||
writer.writerows(user_data[i][c]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks unreadable. let's try to refactor it together
lab3/src/main.py
Outdated
if args.numb_of_rows_filt is True: | ||
output_file.delete_rows_for_filt(args.numb_of_rows_filt) | ||
else: | ||
output_file.filter_by_gender(args.gender_filt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try to use ternary operator here
lab3/src/main.py
Outdated
pdb.set_trace() | ||
|
||
output_file = DataHumans(args.destination_folder, args.file_name) | ||
if args.numb_of_rows_filt is True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to compare var wtih True.
do this:
if args.numb_of_rows_filt is True: | |
if args.numb_of_rows_filt: |
lab3/src/main.py
Outdated
parser.add_argument("--file_name", type=str, required=True) | ||
parser.add_argument("-g", "--gender_filt", type=str) | ||
parser.add_argument("-n", "--numb_of_rows_filt", type=int) | ||
parser.add_argument("--log_level", type=str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the parameter should be positional
lab3/src/main.py
Outdated
parser.add_argument("-g", "--gender_filt", type=str) | ||
parser.add_argument("-n", "--numb_of_rows_filt", type=int) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they should be mutually exclusive
lab3/src/main.py
Outdated
# использовать argparse | ||
parser = argparse.ArgumentParser() | ||
parser.add_argument("--destination_folder", type=str, required=True) | ||
parser.add_argument("--file_name", type=str, required=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the parameter must have default value. not required
lab3/tests/unit/test_lab3_main.py
Outdated
# with patch("builtins.open", mock_open(read_data="data")) as mock_file: | ||
# res = dh.read_data_from_file() | ||
# mock_file.assert_called_with(dh.previous_output, "r", encoding="utf-8") | ||
# assert res == ['test'] No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add blank line at the end of the file
I make all