HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Prompting the user for plot parameters

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theuserpromptingforplotparameters

Problem

Below is the current working code for an ongoing project.

I am wondering if there is anything I can do to improve the code snippet below. My main concerns lie to two area. First being that I append the answer of an input to a list called inputList and then call it in a block of conditionals. I am wondering if this in particular is a good approach. Are there any better approaches?

Secondly, similar concerns lay with how I defined the answers and dictionaries of their variants. My main purpose there is that, regardless of how you enter "yes" or "no", as long as it has those letters and in the proper order, i.e "nO" or "yES", it would be accepted. I also did it as I did so that I would not have to concern myself on checking whether or not the first letter of any of those two words are capitalised or not.

Again, my question being, is this a "solid" approach? What alternative approaches would be better for this snippet of code? Perhaps a better question would be, how can I improve this snippet?

I am also interested in seeing if I can shorten it as much as possible. Similar to how permLet() is written.

```
from itertools import product, zip_longest
import pylab as plot
import numpy as number
import pprint

inputFile = input('File Name: ')

def yLimits():
yMin = int(input('Set lower limit: '))
yMax = int(input('Set upper limit: '))
labelLocation = number.arange(len(count))
plot.bar(labelLocation, list(count.values()), align='center', width=0.5)
plot.xticks(labelLocation, list(count.keys()))
plot.xlabel('Characters')
plot.ylabel('Frequency')
plot.ylim(yMin, yMax)
plot.show()

def ylimChoice():

#returns all permutations of letter capitalisation in a certain word.
def permLet(s):
return(''.join(t) for t in product(*zip(s.lower(), s.upper())))

inputList = []
yesNo = input('Would you like to set custom ylim() arguments? ')
inputList.append(yesNo)
yes = list(permLet("yes"))
no = list(permLet("no")

Solution

It does not improve readability to rename modules. There are a few common aliases used, usually to shorten the name: import numpy as np and import matplotlib.pyplot as plt. Apart from that you should stick to the module name.

I was searching your whole code for the initialization of plot and number, because they sound more like normal variables and not like modules. Especially plot sounds like an object.

You should also choose more descriptive variable names (and adhere to PEP8, Python's official style-guide). capitalization_permutations tells you immediately what the function does, permLet does not.

To iterate over the lines of a file, it is sufficient to do:

with open(inputFile, 'r') as info:
for character in info:
character = character.upper()
# do something

But since you actually want the characters, you could just use collections.Counter here:

with open(inputFile, 'r') as info:
    count = collections.Counter(info.read().upper())


Instead of value = pprint.format(count); print(value) you can directly do pprint.pprint(count).

Currently your main code is all over the place. inputFile is defined in the beginning, followed by the function definitions, followed by more code.

Instead follow this build-up:

  • Module imports



  • CONSTANTS



  • Class definitions



  • function definitions



  • rest of the code



You could put all your code into a main function and call it like this:

def main():
    inputFile = input('File Name: ') # Delete whitespace at end of line here
    with open(inputFile, 'r') as info:
        count = collections.Counter(info.read().upper())
    pprint.pprint(count)
    ylimChoice(count)

if __name__ == "__main__":
    main()


This allows you to do e.g. import plot_parameters to use plot_parameters.ylimChoice() in another script without the main part of the code being run.

At the same time you can remove the reliance on the global variables by just passing the relevant object as parameters (e.g. count to ylimChoice and yCounts).

Lastly this monstrosity:

inputList = []
yesNo = input('Would you like to set custom ylim() arguments? ')
inputList.append(yesNo)
yes = list(permLet("yes"))
no = list(permLet("no"))

if any(yesNo in str({y: y for y in yes}.values()) for yesNo in inputList[0]):
    ...


What you want to know is if the user-choice is some permutation of yes with random capitalization. This is easiest achieved with if user_choice.lower() == 'yes'. At the same time you append the user_value to a list, and then iterate over the first element of that list to use in an any (with one element)(!). That's just too much overhead. Just use this:

user_input = input('Would you like to set custom ylim() arguments? ')
if user_input.lower() in ('y', 'yes'):
    yLimits(count)
else:
    ...
    # do other stuff


Note that I also allowed the user to just type y. Also, the other code will run on all other input. Your code does nothing (not even a notice that the user input was not recognized) if a wrong word is entered.

Code Snippets

with open(inputFile, 'r') as info:
    count = collections.Counter(info.read().upper())
def main():
    inputFile = input('File Name: ') # Delete whitespace at end of line here
    with open(inputFile, 'r') as info:
        count = collections.Counter(info.read().upper())
    pprint.pprint(count)
    ylimChoice(count)

if __name__ == "__main__":
    main()
inputList = []
yesNo = input('Would you like to set custom ylim() arguments? ')
inputList.append(yesNo)
yes = list(permLet("yes"))
no = list(permLet("no"))

if any(yesNo in str({y: y for y in yes}.values()) for yesNo in inputList[0]):
    ...
user_input = input('Would you like to set custom ylim() arguments? ')
if user_input.lower() in ('y', 'yes'):
    yLimits(count)
else:
    ...
    # do other stuff

Context

StackExchange Code Review Q#142375, answer score: 3

Revisions (0)

No revisions yet.