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

Generalized csv reader #22

Open
wants to merge 12 commits into
base: medAngel
Choose a base branch
from
Open

Generalized csv reader #22

wants to merge 12 commits into from

Conversation

seanflyyy
Copy link

No description provided.

seanflyyy added 12 commits December 23, 2021 22:20
changed the csv header names

updated write_to_csv to determine the headers for the csv file from the config.yml when it is writing to csv
Addressed comments about reading the config file once and also indentation issues
…st". also, deleted some irrelevant code. addressed line 177 issue. reassigned yaml_config variable on line 208 to be the output of yaml.full_load.
…ictionaries containing the key value pair of the column name and the corresponding value from the row. adds generate_column_key_value_pair function which creates the key value pair of the column name and the corresponding value from the row. adds check_enum_columns_input_valid function which checks whether input value is within the allowed values specified in the config.yml file. adds config.yml format of inputting enum column, special columns and text columns data
…csv_row and generate_row functions from models.py and write_to_csv function from angel.py to write data into new csv file in the same column order that the input csv file is in
…ns data to ensure they do not match other player’s data. improves on error handling message when there is invalid input for enum columns. removed house requirement code.
@nicktohzyu
Copy link
Member

merge conflict

@nicktohzyu
Copy link
Member

Error value and column should be quoted
image

@nicktohzyu
Copy link
Member

ensure consistency between

  • whether the code expects titles in the first row of the csv files
  • what the csv files you provide actually contain

@nicktohzyu
Copy link
Member

do not commit .DS_store

@@ -1,14 +1,57 @@
# These track the column number of each player attribute in playerlist.csv file
# Note that change in csv column order can be edited here.
player_attribute_index:
Copy link
Member

Choose a reason for hiding this comment

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

Is this still relevant?

@@ -75,131 +78,186 @@ def read_csv(filename):
return person_list


def convert_to_player(row):
def generalized_convert_to_player(row, column_names_index_dict):
Copy link
Member

Choose a reason for hiding this comment

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

why does the name contain "generalized"? is there still a relevant non-generalized version?

generate_column_key_value_pair(text_columns, row) +
generate_column_key_value_pair(enum_columns, row, check_enum_columns_input_valid))

print("output is\n", player_data)
Copy link
Member

Choose a reason for hiding this comment

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

please comment out debugging print statements. Or set a global "debug" variable, and only print if debug is turned on

def generate_column_key_value_pair(type_of_column, row, function=None):
output = []
for column_name in type_of_column:
# obtains the value of passing the column_name as a key to the type of column
Copy link
Member

Choose a reason for hiding this comment

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

??

if function is not None:
valid = function(type_of_column, column_dict, row_input.strip().lower())
if not valid:
# outputs the element of which person that is not within the valid range for that
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, i'm really struggling to understand the comments here

return Player(player_data)


def generate_column_key_value_pair(type_of_column, row, function=None):
Copy link
Member

Choose a reason for hiding this comment

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

I think "type_of_column" is an inappropriate name

else:
row_input = row[column_dict['index']]
if function is not None:
valid = function(type_of_column, column_dict, row_input.strip().lower())
Copy link
Member

Choose a reason for hiding this comment

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

row_input.strip().lower() is called multiple times; it can be abstracted

Copy link
Member

Choose a reason for hiding this comment

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

I don't quite see why you need to pass check_enum_columns_input_valid as a parameter. It seems to be the only case where this parameter is ever used?

output = []
for column_name in type_of_column:
# obtains the value of passing the column_name as a key to the type of column
column_dict = type_of_column[column_name]
Copy link
Member

Choose a reason for hiding this comment

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

What is the type of column_dict? Please use a variable name that is reflective of it


player_data = (generate_column_key_value_pair(special_columns, row) +
generate_column_key_value_pair(text_columns, row) +
generate_column_key_value_pair(enum_columns, row, check_enum_columns_input_valid))
Copy link
Member

Choose a reason for hiding this comment

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

I don't see why you would handle the different kinds of columns using the same function. Why not have three separate functions?

  • handle special
  • handle text
  • handle enum

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.

Generalized player/csv reader through config Update csv reader to only read config once
2 participants