Skip to content
This repository has been archived by the owner on May 5, 2021. It is now read-only.

Channel report #87

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

cablemapper
Copy link

My first "pull request", hopefully doing what I want it to do. The project I am working on is ch_report. The goal is to replace a large VBA enabled spreadsheet with a "flask" enabled dialog box. At this point in time it merely converts the spreadsheet to a python data-base (One time use) and creates a tkinter form. The tkinter form has no functionality as yet and the final product may change as it develops.

[1, 2, 'FIZZ', 4, 'BIZZ', 'FIZZ', 7, 8, 'FIZZ', 'BIZZ', 11, 'FIZZ', 13, 14, 'FIZZBIZZ']
"""
# Test for a valid lst
if not type(lst) is list:
Copy link
Contributor

Choose a reason for hiding this comment

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

isinstance is a nice feature to use here. Example:
if not isinstance(lst, list):

The advantage of isinstance is that you can use it also with classes or tuples that you make along with types. for example:

class Circle():
    pass

my_circle = Circle()
if isinstance(my_circle, Circle)
    print('its a circle')
else:
    print("not a circle")

# Close the pickle
file_obj.close()

except:
Copy link
Contributor

Choose a reason for hiding this comment

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

try to be specific with your excepts such as except FileNotFoundError: After you catch that specific exception you can then do something that keeps the program going as oppose to just exiting.

# Open the pickle
try:
# Open the pickle
file_obj = open("/home/larry/UW/Play/donors", 'wb')
Copy link
Contributor

Choose a reason for hiding this comment

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

When working with file paths, python has some functions that are super nice so that things can be run without hardcoding file paths.

In this instance you can do:

import os

file_obj = open(os.path.abspath('donors')), 'wb')

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, you can use the with context manager here:

try:
    with open(os.path.abspath('donors')), 'wb') as file_obj:
        pickle.dump(donors, file_obj)
except.......

# Call menu method based on user choice
my_methods[chc]()

def m1(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to use functions names that say what the function does. m1 doesn't mean anything to another programmer.


# Insist on correct input & give feedback if wrong
while True:
chc = input()
Copy link
Contributor

@mdgregor mdgregor Mar 12, 2017

Choose a reason for hiding this comment

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

on your input command, you might want to specify what you are asking the user to enter. You have this in the comment of your make_tui(): Example | parameter = ["Report", "Thank you", "Quit"] but the user would never know they can just type the name of what they want.

self.c = c
self.h = h
self.w = w
self.b = b
Copy link
Contributor

@mdgregor mdgregor Mar 12, 2017

Choose a reason for hiding this comment

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

You should rename the variable names what you have in the docstring here
self.side_character_of_grid = s` and so forth

Copy link
Contributor

@mdgregor mdgregor left a comment

Choose a reason for hiding this comment

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

Most of your files are at the root of the project instead of inside your folder maleyl. Can you move them to your folder and then re-push. Thanks~

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants