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

Convert star * import to regular import #10

Closed
angelofallars opened this issue Sep 3, 2021 · 1 comment · Fixed by #13
Closed

Convert star * import to regular import #10

angelofallars opened this issue Sep 3, 2021 · 1 comment · Fixed by #13
Assignees

Comments

@angelofallars
Copy link
Collaborator

main.py imports tkinter and time using * imports. This will pose an issue once we add more and more features (and thus modules) to Pomodrip. If a function from Tkinter and another function from a module have the same name, they will clash and cause errors. Changing to regular imports will improve the readability of code as well.

Suggested actions

  • Change from tkinter import * to import tkinter as tk
  • Change from timer import * to just import timer
  • Update the functions that use tkinter to reflect the changes above like StringVar() to tk.StringVar()
@ryanamay
Copy link
Contributor

ryanamay commented Sep 3, 2021

Also cross-reference to this: https://www.python.org/dev/peps/pep-0008/#imports

Neoloopy added a commit that referenced this issue Sep 3, 2021
Changes added
- converted * import to regular import addressing the problems of #10

- added a semi temporary solution for #12  (does not fix the event when the user directly presses the button while variables are set to "00" the moment the application is opened)
possible solution for #12  is that I disable the button while it is set to "00" or "0" that way the code

>		if Timing == 0:
			messagebox.showinfo("Timer", "Time's up! 🎊")

does not conflict with the catch

> 		if Timing == 0 or Timing is None:
			messagebox.showinfo("Error", "Enter a Value.")
@Neoloopy Neoloopy mentioned this issue Sep 3, 2021
@ryanamay ryanamay linked a pull request Sep 3, 2021 that will close this issue
@ryanamay ryanamay removed a link to a pull request Sep 3, 2021
@ryanamay ryanamay linked a pull request Sep 3, 2021 that will close this issue
ryanamay pushed a commit that referenced this issue Sep 3, 2021
Optimization with a couple of fixes.
This merge fixes issues: #7 #10, and #12
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 a pull request may close this issue.

3 participants