4
\$\begingroup\$
import re
import pandas as pd
import numpy as np
import random
tasks = set()
def main():
 get_task()
 get_time()
 day_organization()
 print_random_sentence()
def get_task():
 global tasks
 while True:
 name_of_task = input("What is your task? ").strip()
 if not name_of_task or re.search(r"\d", name_of_task):
 print("You need to write a task(only with letters)")
 continue
 important = input("How match did the task important from Scale 0-10? ").strip()
 if not important or 10 < int(important) or re.search(r"\D", important):
 print("You need to write importance according to your task"
 "(should be greater than or equal to 0 and less than or equal to 10")
 continue
 other = input("did you have other task. (yes or no)? ").strip().lower()
 if other == "yes":
 tasks.add((name_of_task, int(important)))
 continue
 else:
 x = input("the answer need to be yes or no. what is your answer: ").strip().lower()
 if x == "yes":
 continue
 else:
 return tasks.add((name_of_task, int(important)))
def get_time():
 global tasks
 df = pd.DataFrame([{"Name_of_task": t[0], "Important": t[1]} for t in tasks])
 df = df.sort_values(by="Important", ascending=False)
 for index, row in df.iterrows():
 print(f"Task: {row['Name_of_task']}, Importance: {row['Important']}")
 while True:
 importance = np.array(df["Important"])
 time = input("How match time do you have for all of the tasks(in minutes)? ").strip()
 if time:
 time = float(time)
 if len(df) == 1:
 print(time)
 else:
 time_allocated = time * (importance / importance.sum())
 df["Time_allocated"] = time_allocated
 for index, row in df.iterrows():
 print(
 f"Task: {row['Name_of_task']}, Importance: {row['Important']}, Time needed: {row['Time_allocated']:.2f}")
 break
 if not time or re.search(r"^\D$", time):
 continue
def day_organization():
 global tasks
 start_time = input("When do you want to start do your tasks? ").strip().lower()
 print("DAY ORGANIZATION:")
 print(start_time)
 df = pd.DataFrame([{"Name_of_task": t[0], "Important": t[1]} for t in tasks])
 df = df.sort_values(by="Important", ascending=False)
 for index, row in df.iterrows():
 print(f"Task: {row['Name_of_task']}")
def print_random_sentence():
 sentences = [
 "HAVE A GREAT DAY!",
 "GOOD LUCK!",
 "STAY FOCUSED AND KEEP GOING!",
 "YOU'VE GOT THIS!",
 "KEEP UP THE GOOD WORK!",
 "YOUR PLAN IS READY, NOW GO FOR IT!",
 "WISHING YOU A PRODUCTIVE DAY!"
 ]
 print("\n" + random.choice(sentences))
if __name__ == "__main__":
 main()

It works, but I’d like feedback on how to improve it.

How can I make the code more Pythonic and readable?

Are pandas and numpy the right tools here, or is there a simpler approach?

How can I organize the functions and handle user input more cleanly?

toolic
15.1k5 gold badges29 silver badges211 bronze badges
asked Sep 28 at 19:07
New contributor
Itay Yahav is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$

4 Answers 4

8
\$\begingroup\$

Here are the suggestions that immediately come to mind:

Docstrings, Type Hints and Comments

It's very difficult to tell what your program is supposed to do and the title of your post nor any description you provided in your post sheds much light. The code could be greatly enhanced by adding docstrings that describe what the module and functions do and comments. Type hints, of course, would also help.

Consequently, I will have nothing to say about the actual calculations you are making and how they are performed.

The PEP8 Style Guide Conformance

The PEP8 Style Guide for Python Code describes conventions that a well-structured program should follow. With regards to your code:

The order of imports should be:

  1. Standard library imports.
  2. Related third party imports.
  3. Local application/library specific imports.

You should separated the above groups with a single blank line.

Limit all line lengths to 79.

Functions that are considered "private" (i.e. those that a client is not supposed to directly call and which you do not want to be automatically imported if a client codes from some_module import *) should be named with a leading underscore.

Some of your strings are coded with double quotes and others with single quotes. You should try to stick to a single style.

I could go on, but I will just suggest that you read the guide.

Don't Unnecessarily Use global Statements

You have in your functions global tasks statements. Since you are not assigning to variable tasks, these statements are unnecessary.

Improved Validation

In function get_task you are validating variable name_of_task using a regular expression:

 if not name_of_task or re.search(r"\d", name_of_task):
 print("You need to write a task(only with letters)")
 continue

Presumably you want name_of_task to be exclusively composed of alpha characters based on your error message. But you are only checking to make sure that no digits are present. Consequently, you would accept "abc(def)", which I believe you really do not want to do. You could try instead:

 if not name_of_task or re.search(r"\W|\d", name_of_task):
 ...

You are now rejecting any input that has a non-alphanumeric character or a numeric (i.e. a digit) and consequently you will only accept input that is all alpha. If you are dealing exclusively with ASCII characters, then you could use:

 if not name_of_task or re.search(r"[^a-z]", name_of_task, re.I):
 ...

In the same function you have:

 important = input("How match did the task important from Scale 0-10? ").strip()
 if not important or 10 < int(important) or re.search(r"\D", important):
 ...

If the user enters "x" then the call int(important) will raise a ValueError exception. Simply change the order of tests:

 important = input("How match did the task important from Scale 0-10? ").strip()
 if not important or re.search(r"\D", important) or 10 < int(important):
 ...

You will now only be attempting the conversion if important consists of all digits.

You have:

 other = input("did you have other task. (yes or no)? ").strip().lower()
 if other == "yes":
 tasks.add((name_of_task, int(important)))
 continue
 else:
 x = input("the answer need to be yes or no. what is your answer: ").strip().lower()
 if x == "yes":
 continue
 else:
 return tasks.add((name_of_task, int(important)))

If the user enters "no", which should be a valid entry, you erroneously tell the user that "the answer need to be yes or no. what is your answer: ". You probably want instead:

def get_task():
 while True:
 name_of_task = input("What is the name of your task? ").strip()
 if not name_of_task or re.search(r"\W|\d", name_of_task):
 print("You need to write a task name using only letters.")
 continue
 important = input("Specify the task importance with a number in the range 0 - 10? ").strip()
 if not important or re.search(r"\D", important) or 10 < int(important):
 print("The importance should be an integer in the range 0 - 10")
 continue
 tasks.add((name_of_task, int(important)))
 other = input("Do you have another task? (yes or no) ").strip().lower()
 if other == "no":
 return tasks
 if other != "yes":
 print("the answer need to be yes or no.")

Note that I have modified the prompts for greater clarity.

But this is still not particularly user-friendly because if, for example, the user has entered successfully a task name but then provides an invalid value for important, you throw away the task name and the user has to re-enter it even though it had been entered and validated. You should instead modify the code so that for each piece of successive information you want code a separate loop that repeatedly prompts the user for the valid value:

def get_task():
 while True:
 while True:
 name_of_task = input("What is your task? ").strip()
 if not name_of_task or re.search(r"\W|\d", name_of_task):
 print("You need to write a task name using only letters.")
 else:
 break
 while True:
 important = input("Specify the task importance with a number in the range 0 - 10? ").strip()
 if not important or re.search(r"\D", important) or 10 < int(important):
 print("You need to write importance according to your task"
 "(should be greater than or equal to 0 and less than or equal to 10")
 else:
 tasks.add((name_of_task, int(important)))
 break
 while True:
 other = input("Do you have another task? (yes or no) ").strip().lower()
 if other == "no":
 return tasks
 if other == "yes":
 break
 print("The answer needs to be yes or no.")

In function day_organization you ask:

 start_time = input("When do you want to start do your tasks? ").strip().lower()

I had no idea what you were looking for.

Better Variable Names

You have in function get_time:

 while True:
 importance = np.array(df["Important"])

Variable importance is well-named since it suggests a quantity (e.g. [0, 10]) whereas important or Important, which are adjectives, suggest True or False boolean values. Therefore, in function get_task rename variable important to importance and change the data frame column name to "Importance").

answered Sep 28 at 21:10
\$\endgroup\$
4
  • \$\begingroup\$ Instead of flag variables like valid_input you may want to just have a while True: loop and use break to exit from it. \$\endgroup\$ Commented Sep 29 at 22:36
  • \$\begingroup\$ (otherwise +1) Please don't do flag = False; while not flag: ...if cond: flag = True, there's a great break keyword to express the same concept without an extra variable and with much less mental load. And type hints are really, really optional, maybe even harmful for scripts as tiny as this one. \$\endgroup\$ Commented 2 days ago
  • \$\begingroup\$ @Chris Good point! \$\endgroup\$ Commented 2 days ago
  • \$\begingroup\$ Limiting line lengths to 79 characters is not necessarily helpful. I mean, a developer might decide a 79-character line length limit makes sense for them for some particular reason (perhaps as simple as the size of their preferred editor window), and in that case, sure, go ahead and do it, but have a good reason, not just because PEP 8 says so. \$\endgroup\$ Commented 2 days ago
5
\$\begingroup\$

UX

When I run the code, I am prompted with:

What is your task?

But, I have no idea what it is asking me to input. I recommend printing more information prior to the prompt. You could describe what kind of task is expected.

Another input prompt is:

did you have other task. (yes or no)? no
the answer need to be yes or no. what is your answer: no

This is confusing. I answered the prompt with one of the suggested choices ("no"), but then the code responded that it did not accept "no". That logic should be improved.

Typos

This line has a typo:

time = input("How match time do you have for all of the tasks(in minutes)? ").strip()

I think you meant to type "much" instead of "match":

time = input("How much time do you have for all of the tasks (in minutes)? ").strip()

This is an awkwardly-worded question:

important = input("How match did the task important from Scale 0-10? ").strip()

I think this is what you meant to say:

important = input("How important is the task, on a scale 0 to 10? ").strip()

Input checking

Since the code accepts a lot of input from the user, consider using pyinputplus because it has built-in checking.

answered Sep 29 at 0:08
\$\endgroup\$
5
\$\begingroup\$

instance attributes

tasks = set()
def get_task():
 global tasks
 ...
 tasks.add((..., ...))

We need a global if we wish to assign tasks = set( ... ) in a way that code outside the function can see it. Else we would be assigning to a local named tasks, which disappears when we return.

Simply omit that global declaration, as the tasks.add() reference will properly locate that symbol in the top-level module namespace, and then we mutate the existing object.

OOP

But a bigger issue is, maybe we would rather define a class, and then talk about the self.tasks attribute. Whenever you're tempted to write global, consider whether an object would be a better place to keep track of invariant properties, to keep track of what you want to be true in your app.

command line args

The while True: loop seems needlessly interactive.

Consider parsing sys.argv to obtain the required inputs. Or let typer sweat the details for you. And you'll get some nice --help output "for free".

answered Sep 28 at 22:55
\$\endgroup\$
2
\$\begingroup\$

You devote a not insignificant amount of logic in your program to sorting based on priority. Rather than using unsorted data and repeatedly re-sorting it based on priority, you may wish to use a data structure that maintains that priority itself: a priority queue. The difference is likely negligible for trivial data sets, but more noticeable when scaling to very large data sets.

These are typically implemented using binary heaps, and you can find examples in the heapq and queue libraries. The former is probably more relevant as your program does not appear to be bothered with any concurrency concerns.

These will also maintain order of insertion for tasks with the same priority. As you use a set to store your data, this is not guaranteed. Relevant Stack Overflow reading: Why don't Python sets preserve insertion order?

As a small additional note, when you iterate over the rows in your dataframe, you name the index, but you never use it:

 for index, row in df.iterrows():
 print(f"Task: {row['Name_of_task']}, Importance: {row['Important']}")

You'd be better off using _ in these circumstances.

 for _, row in df.iterrows():
 print(f"Task: {row['Name_of_task']}, Importance: {row['Important']}")
answered Sep 29 at 21:06
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.