We need to switch the development branch to use python 3.

[brugger1]

We are currently still building against python 2 on quartz and running the test suite using python 2. The third party libraries on quartz should be rebuilt with python 3 and the config site file updated. The CI should also be built with python 3.

We should remove the python 2 stuff at some later date once we have confidence that the python 3 stuff is solid. This may be in a few months.

[brugger1]

@cyrush, @biagas I built the new third party libraries on quartz and built VisIt against it and ran the test suite and got quite a few failures.

• There are a few random one off failures that need investigation
• There were several failures with query output tests where a dictionary was output and it looks like things were reordered in the dictionary. Those may be rebaselines is that is expected, although I noticed one where something was missing. Would need to look at them all in detail to make sure there isn't an issue.
• The simulation tests all exited with a zero exit code, so something might be amis
• The movie making test failed. Since that involves scripts, perhaps there is an error there

How do you want to proceed? I can give you a tar file with the html output on the CZ.

[biagas]

Sounds like what I saw. You can give me the html output, I am willing to take a look at the results. @cyrush, have the sim tests been passed through the py2to3 filter? Do the sim swig files need updating?

[brugger1]

@biagas, @cyrush I gave you both the test results on the CZ.

[biagas]

Thanks @brugger1. I would say that the dictionary output is now sorted by key. Makes confirming the new pick results a bit challenging. Overall I think the movie and sim tests are the big issues.

[biagas]

@brugger1, is there a log file of test output that I can have?

[brugger1]

@biagas, @cyrush I gave both of you the log files on the CZ.

[biagas]

Thanks, that wasn't what I was looking for. I'm thinking of a log file that captures all the test output. I generate one when I run the tests, but I also use '--lessverbose' to capture more output.

[biagas]

Here's the error output I saw on my run for cinema test:

>>> Traceback (most recent call last):
  File "/usr/WS1/kbonnell/Branches/Pyside_Take2/BuildPascal/bin/visitcinemamain.py", line 39, in <module>
    main()
  File "/usr/WS1/kbonnell/Branches/Pyside_Take2/BuildPascal/bin/visitcinemamain.py", line 33, in main
    cinema.Execute()
  File "/usr/WS1/kbonnell/Branches/Pyside_Take2/BuildPascal/bin/visitcinema.py", line 738, in Execute
    self.CreatePhiThetaDatabase()
  File "/usr/WS1/kbonnell/Branches/Pyside_Take2/BuildPascal/bin/visitcinema.py", line 1322, in CreatePhiThetaDatabase
    f.write('       "default":%d,\n' % ideg(theta[self.theta/2]))
TypeError: list indices must be integers or slices, not float

 - - - - - - - - - - - - - - -
  START:  Test script cinema-a.py

    BEGIN SECTION: Cinema spec A static camera
    Test case 'cinema_0_00' PASSED
    Test case 'cinema_0_01' PASSED
    Test case 'cinema_0_02' PASSED
    Test case 'cinema_0_03' PASSED
    BEGIN SECTION: Cinema spec A phi-theta camera
    Test case 'cinema_1_00' PASSED
    Test case 'cinema_1_01' PASSED
    Test case 'cinema_1_02' FAILED
Traceback (most recent call last):
  File "/usr/WS1/kbonnell/Branches/Pyside_Take2/visit/src/test/tests/hybrid/cinema-a.py", line 201, in <module>
    main()
  File "/usr/WS1/kbonnell/Branches/Pyside_Take2/visit/src/test/tests/hybrid/cinema-a.py", line 197, in main
    test1(db)
  File "/usr/WS1/kbonnell/Branches/Pyside_Take2/visit/src/test/tests/hybrid/cinema-a.py", line 158, in test1
    params = eval(json)
  File "<string>", line 7
    "theta": {
             ^
SyntaxError: unexpected EOF while parsing
>>> 

[biagas]

movie.py is missing () around a print statement that it writes out to a script file. line 99 on the develop version of the file.

[biagas]

tests/hybrid/py_expr_script_00.vpe line 37 uses 'xrange', should be just 'range' ?

[biagas]

I'm making mods to scripts as I find the problems and if I know the fix. Shall I give them to you @brugger1? As I finish each one, or when I have worked through as many as I can?

[cyrush]

Sounds like there is more work to do.

This is one of the down sides of having to turn on several flags to get to the same config that we test ☹

I really wish more things defaulted to on.

-Cyrus

From: Kathleen Biagas notifications@github.com Reply-To: visit-dav/visit reply@reply.github.com Date: Wednesday, January 20, 2021 at 1:10 PM To: visit-dav/visit visit@noreply.github.com Cc: Cyrus Harrison cyrush@llnl.gov, Mention mention@noreply.github.com Subject: Re: [visit-dav/visit] We need to switch the development branch to use python 3. (#5383)

Thanks, that wasn't what I was looking for. I'm thinking of a log file that captures all the test output. I generate one when I run the tests, but I also use '--lessverbose' to capture more output.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/visit-dav/visit/issues/5383#issuecomment-763948264, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAJDUHUBRD37X42NV6MET73S25BEBANCNFSM4WCZDEYQ.

[brugger1]

You can give them to me when you have worked through as many as you can.

[biagas]

@cyrush, how should this line in makemovie.py line 292 be fixed? The error message is TypeError: can only concatenate str (not "bytes") to str

                s = s + os.read(fd_list[0][0], 100)

Does the os.read part just get wrapped in str() ?

[markcmiller86]

Does the os.read part just get wrapped in str() ?

That would certainly fix the error as in...

                s = s + str(os.read(fd_list[0][0], 100))

[biagas]

Thanks @markcmiller86. That's what I did. Now the movie test is hanging, so there must be another issue somewhere that isn't raising an obvious python3 error.

[biagas]

@brugger1 I've given you a tarball with changes that fix python 3 errors for py_expr test, cinema-a test, movie test. The movie test hangs for me, but that could be something to do with my pyside-build. I will look into pluginVsInstall next.

[biagas]

How would I fix this error (seen in simulation test failures):

 File /src/test/visit_test_main.py", line 2422, in consolecommand
    self.p.stdin.write(cmd + "\n")
TypeError: a bytes-like object is required, not 'str'

I believe there are a lot of cases like this (attempting to write a string to stdin or some other PIPE) in the sim tests and the sim support in visit_test_main.py

[markcmiller86]

Ok, that is the opposite direction of conversion.

Maybe try self.p.stdin.write(bytes(cmd+"\n"),"utf_8")) based on...

Help on class bytes in module builtins:

class bytes(object)
 |  bytes(iterable_of_ints) -> bytes
 |  bytes(string, encoding[, errors]) -> bytes
 |  bytes(bytes_or_buffer) -> immutable copy of bytes_or_buffer
 |  bytes(int) -> bytes object of size given by the parameter initialized with null bytes
 |  bytes() -> empty bytes object
 |  
 |  Construct an immutable array of bytes from:
 |    - an iterable yielding integers in range(256)
 |    - a text string encoded using the specified encoding
 |    - any object implementing the buffer API.
 |    - an integer
 |  
 |  Methods defined here:
 |  
 |  __add__(self, value, /)
 |      Return self+value.
 |  

[biagas]

@brugger1 I gave you another file with fixes for pluginVsInstall tests.

[biagas]

For queries tests: scf.py pick.py conncomp.py pickNamedArgs.py queriesOverTime.py

I checked the failing tests (copied current vs baseline results to python3 and performed an equivalence test). They can all be rebaselined.

[biagas]

@cyrush, can you look into locus.py. Here's the error message I see:

Traceback (most recent call last):
  File "/src/test/tests/hybrid/locus.py", line 42, in <module>
    import visit_writer
ImportError: dynamic module does not define module export function (PyInit_visit_writer)
>>>

[biagas]

Maybe try self.p.stdin.write(bytes(cmd+"\n"),"utf_8")) based on...

Seems like a lot of extra work to write a simple string :( Do we always want to use "utf_8"? Any instance where we would want "ascii"? If default encoding is 'utf_8' does it need to be specified each and every time you want to encode a string as bytes?

I've seen some examples using 'encode' or 'decode' methods on the string. Which is better?

What about this section of code, looking for a string in the output of readline?

     buf = sim.p.stderr.readline()
        print(buf)
        if "Command '%s'"%com in buf:
            keepGoing = False

Should 'buf' be decoded? Or the string 'encoded' ?

[cyrush]

Are these changes on a branch we can attack together?

For some of these strings vs bytes issues, there are file mode open options that can solve them. We shouldn't have to do too much manual encode / decode steps.

[biagas]

if @brugger1 doesn't have one already, I can get one started.

[brugger1]

I just pushed the branch. There's not much on it, just the updated config site file. It's

task/brugger1/2020_01_19_upgrade_tpls

[biagas]

Great! So, do we branch off your branch, or just check it out? I will add some of the fixes I created yesterday that I know work.

[brugger1]

I'm not sure. It seems whatever is simpler.

[brugger1]

@biagas, have you made any changes. If not, I'll rebaseline the queries tests you mentioned.

[biagas]

@brugger1 I was waiting for the build to finish so I can verify. Go ahead and rebaseline, and I will commit when you are done.

[biagas]

I added the fixes for py_exprs, cinema-a and pluginVsInstall tests. Investigating hang with movie.py

[brugger1]

I investigated the databases/ffp.py failure and it was because I didn't copy libstripack.so to build/lib. This is a non-issue.

[brugger1]

Looking at databases/FMS.py failure now.

[brugger1]

The databases/FMS.py failure was from a python conversion issue. I suspect this was added after the test suite was converted to python 3. I fixed the error and pushed the changes.

[markcmiller86]

Regarding any conversion between Python 2/3, should we continue to try to ensure the resulting code works in both or just abandon Python 2 altogether?

[brugger1]

I looked at unit/launcher.py and the failure is

Traceback (most recent call last): File "/usr/WS1/brugger/visit_dev_git/visit/src/test/tests/unit/launcher.py", line 143, in output = FormatLauncherOutput(GetLauncherCommand([visit] + args)) File "/usr/WS1/brugger/visit_dev_git/visit/src/test/tests/unit/launcher.py", line 56, in GetLauncherCommand ru = stdout.find("RUN USING") TypeError: argument should be integer or bytes-like object, not 'str'

It looks similar to what @biagas encountered earlier with some proposed solutions. What did you go with?

[markcmiller86]

What did you go with?

Saw from @cyrush that we may need to check open() calls to ensure ascii rather than binary mode is active.

[biagas]

try: ru = stdout.find("RUN USING".encode())

[biagas]

Also, launcher.py has some entries like 'string.replace' that need to be changed to 'str.replace'.

[cyrush]

Regarding any conversion between Python 2/3, should we continue to try to ensure the resulting code works in both or just abandon Python 2 altogether?

It's not very hard to have code that supports both. I'd opt for having both work.

[brugger1]

Yes, I saw the str.replace issue, as well as a few others.

[cyrush]

can you put up a draft PR for our branch? I think I can provide some feedback easily that way.

[markcmiller86]

I'd opt for having both work.

I am inclined to agree. However, I think that means we should test both too. If we don't, for sure one will fall out of working order.

[markcmiller86]

Maybe we can alternate nights running python2 on even numbered days and python 3 on odd numbered days

[brugger1]

I fixed unit/launcher.py and pushed it. I also created a draft pull request.

[brugger1]

I believe we still have the following failures: hybrid/movie.py hybrid/locus.py simulation/*.py

[biagas]

@brugger1 is correct, and I am working on the movie issue.

[brugger1]

@cyrush, I was looking into the locus.py failure:

Traceback (most recent call last): File "/src/test/tests/hybrid/locus.py", line 42, in import visit_writer ImportError: dynamic module does not define module export function (PyInit_visit_writer)

and looked at ./tools/data/writer/py_visit_writer.c

and found

#if __GNUC__ >= 4
/* Ensure this function is visible even if -fvisibility=hidden was passed */
PyObject * __attribute__ ((visibility("default"))) PyInit_visit( void )
#else
PyObject * PyInit_visit( void )
#endif

I changed "PyInit_visit" to "PyInit_visit_writer" and that didn't fix it. I did a make and it compiled and linked it into lib/visit_writer.so. Any other ideas?

[brugger1]

I fixed the python issue with the simulation tests. I tried 2 simulation tests to verify the fix and I noticed some text output from one of the tests had more significant digits in the text output, so there may need to be some rebaselines. I pushed the changes.

[brugger1]

I have a partial fix for tests/hybrid/movie.py. The remaining problem is in makemovie.py. I pushed my changes.