More python 2/3 issues

[egelmex]

fixing dict_keys is not subscriptable.
work towards #5

[jadudm]

So, while I like the idea of merging things in, I'd actually like it, for something like this, I have a question. For example, I don't

1. have a working build.
2. have a system with both Python 2 and Python 3 installed.
3. have any instructions for how to test this merge request, given that it involves running a build with different Pythons active.

It seems like these are small changes that are part of a larger pull request; that is, there should be a series of commits terminating in one merge request, and that merge request should fix the Python 2/3 problem completely. Reviewing a whole bunch of one-line merge requests feels awfully tedious.

Given that we're establishing process, I'm just trying to understand how (for something like this) I'm supposed to support the process, given that I don't have everything in place yet. (I'm still trying to get the 32/64-bit issues worked out, so I have a reliable TVM-based build.

[egelmex]

Sorry this was my bad, I meant to comment on this last night as I found a bunch more python3 issues to leave this pull request for now.

In regards to your points

• have a working build.
  • This code is invoked before things start breaking
• have a system with both Python 2 and Python 3 installed.
  • That is ok, leave it for someone who does. I have two systems with different pythons installed. On deb you may be able to use update alternative. (python 2/3 handling is messy at the moment http://askubuntu.com/questions/14615/how-do-i-make-the-terminal-run-python-3-1)
• have any instructions for how to test this merge request, given that it involves running a build with different Pythons active.
• This is a good point. I will try include instructions on testing this when I get a chance, got to fix the other python3 issues I found first.

[egelmex]

It seems like these are small changes that are part of a larger pull request; that is, there should be a series of commits terminating in one merge request, and that merge request should fix the Python 2/3 problem completely. Reviewing a whole bunch of one-line merge requests feels awfully tedious.

On this note, you can review commits in a pull request, so if you leave a comment you don't have re-review code. Pull requests can be updated in situ, with more work as it's done. What you suggested is ideal, however I like to commit regularly and get feedback if early before too much work need redoing if choices were bad.

[jadudm]

Sure thing.

In case it wasn't obvious, this was more a learning question for me, and
not really about a subscript fix on one line of python. :-)

Thank you.
On Apr 25, 2013 4:12 AM, "Martin Ellis" [email protected] wrote:

It seems like these are small changes that are part of a larger pull
request; that is, there should be a series of commits terminating in one
merge request, and that merge request should fix the Python 2/3 problem
completely. Reviewing a whole bunch of one-line merge requests feels
awfully tedious.

On this note, you can review commits in a pull request, so if you leave a
comment you don't have re-review code. Pull requests can be updated in
situ, with more work as it's done. What you suggested is ideal, however I
like to commit regularly and get feedback if early before too much work
need redoing if choices were bad.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/11#issuecomment-16993751
.

[egelmex]

I have a logic error here somewhere I think...
building kroc (kroc not tvm flavour) with python3

make[2]: Entering directory `/home/me92/src/kroc-git/kroc-git/modules/bsclib/libsrc/filelib'
/home/me92/src/kroc-git/kroc-git/tools/kroc/occbuild --verbose --in-tree /home/me92/src/kroc-git/kroc-git --toolchain=kroc --library file.lib --include filelib.inc cfile.o filelib.tce
occbuild: Setting ISEARCH to: /home/me92/src/kroc-git/kroc-git/modules/inmoslibs/libsrc/forall:/home/me92/src/kroc-git/kroc-git/tvm/posix
occbuild: Running command: /home/me92/src/kroc-git/kroc-git/tools/kroc/kroc --in-tree /home/me92/src/kroc-git/kroc-git -I/home/me92/src/kroc-git/kroc-git/modules/inmoslibs/libsrc/forall -L/home/me92/src/kroc-git/kroc-git/modules/inmoslibs/libsrc/forall -I/home/me92/src/kroc-git/kroc-git/tvm/posix -L/home/me92/src/kroc-git/kroc-git/tvm/posix --incpath
occbuild: Running command: ar rc liboccam_file.a filelib.o cfile.o
ar: filelib.o: No such file or directory
occbuild: Command failed: ar rc liboccam_file.a filelib.o cfile.o

[egelmex]

I could do with some help converting this one into a keys= statement.

[atsampson]

Apologies for the delayed response -- I don't think github notified me about the inline comment!

The way to fix that one is to turn compare_symbols into a function that returns the key for a symbol. You could also use enumerate() rather than the explicit counter. So something like (untested):

def enumerate_symbols(symbols):
    def sort_key(symbol):
        prio = int(symbols[symbol].get("PRIO", 0))
        return (prio, symbol)

    for i, symbol in enumerate(sorted(symbols.keys(), key=sort_key)):
        symbols[symbol]["offset"] = i

[atsampson]

That also leaves only one call to safe_sorted, so you could probably just use list(sorted(...)) at the bottom too.