Skip to content

Conversation

@asarunova
Copy link

Fixed the parser for actor data - made it more readable and compatible with panda data frames.

actors_df = convert_to_df(actors_data)

# Display the frame
from IPython.display import display
Copy link
Collaborator

Choose a reason for hiding this comment

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

This display variable is unused in the rest of the program.

Also I'd recommend to move all your imports to the top of the file for better readability

)
args = argparser.parse_args()

main(args.file, args.out) No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would also be nice to include an example log (txt) file that someone can run using this simple test just to see what the outputs look like


actors_key: str = "Actors"
data[actors_key] = {}
data[actors_key]["Id"] = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a fine change, but as is you'll need to fix the example.py script (Line ~470) which still uses the "Id" method to index into a dictionary containing "Location" and "Rotation". Can you please fix that?

assert min(lens) == max(lens) # all lengths are equal!
# NOTE: pandas can't haneld high dimensional np arrays, so we just use lists
# all lengths are equal!
if not min(lens) == max(lens):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure what the benefit of this is over a simple assert. Requires users to have the ipdb module loaded and will issue a nasty ModuleNotFoundError which might confuse the users.

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.

3 participants