coding-blocks / content-downloader

Python package to download files on any topic in bulk.
MIT License
8 stars 25 forks source link

gui for content downloader #5

Closed akansh97531 closed 7 years ago

akansh97531 commented 7 years ago

gui opens with command : python tk.py

championswimmer commented 7 years ago

@nikhilkumarsingh review

nikhilkumarsingh commented 7 years ago

Awesome work @akansh97531 ! Some minor improvements/features are required:

  1. Rename tk.py to something else (maybe gui.py) since keeping names same as built-in modules/functions is not a good practice.
  2. Improve formatting. Keep space after comma and around operators.
  3. Remove redundant imports in tk.py. Keeping just from tkinter import * is enough. Also, make this import Python 2 and 3 compatible. Also, explain the reason behind this import: from multiprocessing import Queue in gui_download.py.
  4. Add a cli command to open gui.
  5. Add application-icon for gui.
akansh97531 commented 7 years ago

okay sir i will work on 1,2,4,5 , and there is no need of : from multiprocessing import Queue in gui_download.py i was trying something so , i forgot to remove it @nikhilkumarsingh

akansh97531 commented 7 years ago

all improvements done with support of python2 @nikhilkumarsingh

nikhilkumarsingh commented 7 years ago

Nice work @akansh97531

  1. Please complete the task 2 as well since it makes code more readable and maintainable.
  2. Also, you can set the entry point for gui.py as ctdl-gui .
  3. Please follow the code for clix to understand how to add icon in a Tkinter application. (You can use a png image and will have to mention file path in MANIFEST.in like this otherwise it will not get recognized by package manager.)
akansh97531 commented 7 years ago

changes done ! @nikhilkumarsingh

nikhilkumarsingh commented 7 years ago

Very poor formatting @akansh97531 . Simply find and replace will not do. See this script as reference. Don't make hurry in this task. It is only for readability and will help in longer run.

nikhilkumarsingh commented 7 years ago
  1. You haven't deleted tk.py in your code.
  2. Rename gui_download.py to gui_downloader.py
  3. Upload any icon of your choice for now.
akansh97531 commented 7 years ago

please review the formatting now , added some comments to help understand better if you want any more changes please tell me @nikhilkumarsingh

nikhilkumarsingh commented 7 years ago

The only problem I have is with the spaces and commas. Please use the style as used in this script . You have used spaces unnecessarily at some places and never used it at others. Follow the reference script strictly.

akansh97531 commented 7 years ago

@nikhilkumarsingh i have done it manually now

nikhilkumarsingh commented 7 years ago

The code looks much better now.

  1. Remove useless spaces after ( and before ) .
  2. Rename the ctdl gui window from Content Downloader to ctdl only.
  3. Make a consistent format for each function, i.e:
    def <func-name>(<arguments>):
          """
          <short-description of function>
          """
          <global varibales>
          .....
akansh97531 commented 7 years ago

@nikhilkumarsingh done!!

akansh97531 commented 7 years ago

@nikhilkumarsingh please review

nikhilkumarsingh commented 7 years ago

All looks fine @akansh97531 Update the readme with info about ctdl-gui. You can also add a gif showing usage of ctdl-gui.

akansh97531 commented 7 years ago

@nikhilkumarsingh added info and gui

nikhilkumarsingh commented 7 years ago

I am getting this error:

Traceback (most recent call last):
  File "/usr/local/bin/ctdl-gui", line 11, in <module>
    load_entry_point('ctdl==1.4.5', 'console_scripts', 'ctdl-gui')()
  File "/usr/local/lib/python2.7/dist-packages/pkg_resources/__init__.py", line 560, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/usr/local/lib/python2.7/dist-packages/pkg_resources/__init__.py", line 2648, in load_entry_point
    return ep.load()
  File "/usr/local/lib/python2.7/dist-packages/pkg_resources/__init__.py", line 2302, in load
    return self.resolve()
  File "/usr/local/lib/python2.7/dist-packages/pkg_resources/__init__.py", line 2308, in resolve
    module = __import__(self.module_name, fromlist=['__name__'], level=0)
  File "/usr/local/lib/python2.7/dist-packages/ctdl/gui.py", line 40, in <module>
    img = Image("photo", file="icon.gif")
  File "/usr/lib/python2.7/lib-tk/Tkinter.py", line 3329, in __init__
    self.tk.call(('image', 'create', imgtype, name,) + options)
_tkinter.TclError: couldn't open "icon.gif": no such file or directory

when I open ctdl-gui from a directory other than the package directory. It seems to be a problem of relative imports. Please fix it.

akansh97531 commented 7 years ago

okay i will look into it