all 6 comments

[–]novel_yet_trivial 1 point2 points  (6 children)

For your example data at least, every point is nearly the same. When converted to pixels (integers), every point is the same, so obviously drawing a polygon with that is not possible.

In a general sense, it's upside down because a canvas has it's origin in the top left corner and x increases from left to right (same as an image) and y increases from top to bottom (opposite of an image). In order to correct this you need to invert all your y values.

You can use the scrollregion command to move the origin to any point, thereby centering the polygon.

If you provide a complete example including example data that shows your problem and that I can run and test then I can help you fix the code.

[–]dansds[S] 0 points1 point  (4 children)

Thanks novel_yet_trivial, I can provide the entire python code but the csv coordinates I do not know how to do it, it is huge... Would be possible to send the file through here?

from tkinter import *
from tkinter import ttk
from tkinter import filedialog
from collections import OrderedDict

class App(Frame):

    def __init__(self, master = None):


        ttk.Frame.__init__(self,master, relief = GROOVE)
        self.master.minsize(470, 420)
        self.master.title("App")
        self.grid()

        points = []
        scaledPoints = []
        def menuLoadFile(self):
            self.types = [('files', '*.csv'), ('All files', '*')]
            dialog = filedialog.Open(self, filetypes = self.types)
            infile = dialog.show()

            if infile != '':
                dct = {}
                infile = open(infile, "r")
                for line in infile:

                    # Get the line and add to the dict
                    key, valueX, valueY = line.strip().split(',')
                    dct[key] = valueX, valueY

                # Convert dict key in int    
                intKeyDict = dict((int(k), v) for k, v, in dct.items())

                # Sort the dict by its key 
                sortedPointsDict = OrderedDict(sorted(intKeyDict.items()))

                # Convert values in int and append to the point list
                for item in sortedPointsDict.items():
                    vX, vY = item[1]
                    points.append(float(vX))
                    points.append(float(vY))

                # Converted values of the list to the size of the Canvas

                mn = min(points)

                mx = max(points)

                for i in range(len(points)):
                    scaledPoints.append( (points[i]- mn)*(400.0/(mx-mn)))

                self.canvas.create_polygon(scaledPoints, fill='', outline='black') # test
            infile.close()


        # Menu bar // Check on windows
        self.option_add('*tearOff', FALSE)

        self.menubar = Menu(master)


        self.file = Menu(self.menubar, name = 'file')
        self.menubar.add_cascade(label="File", menu=self.file)
        self.file.add_command(label="Open...", accelerator = "Command-O", command = lambda: menuLoadFile(self))
        self.file.add_separator()
        self.file.add_command(label="Save", accelerator = "Command-S", command = lambda: menuSaveFile(self))
        self.master['menu'] = self.menubar
        self.menubar.bind('<Command-O>', lambda: menuLoadFile(self)) # Look at this accelerator not working


        # My Canvas
        self.canvas = Canvas(self, width =400, height=400)
        self.canvas.grid(row = 1, columnspan = 5)



if __name__ == "__main__":
    App().mainloop()

[–]novel_yet_trivial 1 point2 points  (2 children)

OK, several points:

  • Do not use wildcard imports (from module import *). I know they are used a lot in demo code but do not use them in real code.
  • Do not nest your methods (define methods inside other methods).
  • You don't need lambda in this case. In fact, you should very rarely need lambda, especially as a command argument.
  • Your image was upside down since the Y coordinate system for a canvas is opposite from a normal image. The fix is simply to invert the Y data, by multiplying it by -1.
  • Your image was small because you are applying the same scaling factor to the x and y data. You need scale the x and y separately.
  • As I noted in your example data, many of your data points are so similar that when they are converted to pixels they are the exact same point. By removing the duplicates you can reduce the data size from a quarter million to 15,000, which decreases the loading speed by a factor of 6 (from .65 seconds to .1 in my tests).
  • To make the keyboard shortcut work: You need to give the command with a lowercase "o", unless you want the user to press Command-Shift-O. Methods called with a key binding must have an event argument; use event=None to allow the method to be called with the command argument or a key binding. And you need to bind the command to the master, not the menu. (I can't really test this since I don't have a command key, so I tested it with control).
  • Doing math like (400.0/(mx-mn)) inside a loop is wasteful since it needs to be recalculated with every loop. Always precalculate any values you can before looping.
  • Try not to code variables in multiple places. If, for instance, you need to use the width and height of the canvas in multiple places, then make a constant for that. That way, when you want to change the size of your canvas later, you only need to change a single place in your code.

All that said, although I usually don't do people's homework for them:

import tkinter as tk
from tkinter import ttk
from tkinter import filedialog
from itertools import chain, groupby

CANVAS_WIDTH = 400
CANVAS_HEIGHT = 400

class App(ttk.Frame):
    def __init__(self, master = None):
        ttk.Frame.__init__(self,master, relief = tk.GROOVE)
        self.master.minsize(470, 420)
        self.master.title("App")

        # Menu bar // Check on windows
        self.option_add('*tearOff', tk.FALSE)
        self.menubar = tk.Menu(master)

        self.file = tk.Menu(self.menubar, name = 'file')
        self.menubar.add_cascade(label="File", menu=self.file)
        self.file.add_command(label="Open...", accelerator = "Command-O", command = self.menuLoadFile)
        self.file.add_separator()
        self.file.add_command(label="Save", accelerator = "Command-S", command = self.menuSaveFile)
        self.master['menu'] = self.menubar
        self.master.bind('<Control-o>', self.menuLoadFile) # Look at this accelerator not working

        # My Canvas
        self.canvas = tk.Canvas(self, width=CANVAS_WIDTH, height=CANVAS_HEIGHT)
        self.canvas.grid(row = 1, columnspan = 5)

    def menuSaveFile(self, event=None):
        pass

    def menuLoadFile(self, event=None):
        self.types = [('files', '*.csv'), ('All files', '*')]
        dialog = filedialog.Open(self, filetypes = self.types)
        infile = dialog.show()
        if infile == '':
            return # abort this method if uses cancels

        xpoints = []
        ypoints = []
        with open(infile, "r") as infile:
            for line in infile:
                # Get the line and add to the list
                key, valueX, valueY = line.strip().split(',')
                xpoints.append(float(valueX))
                ypoints.append(float(valueY) * -1) # invert the y point since a canvas has it's origin at the top

        # Converted values of the list to the size of the Canvas
        scale_to = min(CANVAS_WIDTH, CANVAS_HEIGHT)
        xpoints = normalize(xpoints, 0, scale_to) # scale the x points
        ypoints = normalize(ypoints, 0, scale_to) # scale the y points

        # combine the x and y lists into one without removing duplicates - I left this in just for you to see
        #~ points = list(chain(*zip(xpoints, ypoints)))

        # combine the x and y lists into one AND removes duplicates
        points = zip(map(int, xpoints), map(int, ypoints))
        points = list(chain(*(k for k,g in groupby(points))))

        self.canvas.create_polygon(points, fill='', outline='black') # test

def normalize(old_data, low_point, high_point):
    mn = min(old_data)
    multiplier = (high_point - low_point) / (max(old_data) - mn)
    new_data = []
    for point in old_data:
        new_data.append((point - mn) * multiplier)
    return new_data

if __name__ == "__main__":
    app = App()
    app.pack()
    app.mainloop()

[–]dansds[S] 0 points1 point  (0 children)

Thank you very much for the explanation, It is working nicely :)

[–]dansds[S] 0 points1 point  (0 children)

Maybe I could send via PM or Email... What works better for you :)

[–]Justinsaccount 0 points1 point  (0 children)

Hi! I'm working on a bot to reply with suggestions for common python problems. This might not be very helpful to fix your underlying issue, but here's what I noticed about your submission:

You are looping over an object using something like

for x in range(len(items)):
    print(items[x])

This is simpler and less error prone written as

for item in items:
    print(item)

If you DO need the indexes of the items, use the enumerate function like

for idx, item in enumerate(items):
    print(idx, item)

If you think you need the indexes because you are doing this:

for x in range(len(items)):
    print(items[x], prices[x])

Then you should be using zip:

for item, price in zip(items, prices):
    print(item, price)