gainsols.py contains py2 only syntax

[gijzelaerr]

hi! While making a Cubical python3 package I encountered the following issue:

/usr/lib/python3/dist-packages/cubical/data_handler/ms_tile.py:960: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if model_source is 1:
/usr/lib/python3/dist-packages/cubical/database/casa_db_adaptor.py:486: SyntaxWarning: "is" with a literal. Did you mean "=="?
  do_export = (self.mode is "create")
/usr/lib/python3/dist-packages/cubical/database/pickled_db.py:73: SyntaxWarning: "is" with a literal. Did you mean "=="?
  assert (self.mode is "create")
/usr/lib/python3/dist-packages/cubical/database/pickled_db.py:94: SyntaxWarning: "is" with a literal. Did you mean "=="?
  assert (self.mode is "create")
/usr/lib/python3/dist-packages/cubical/database/pickled_db.py:114: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if self.mode is "create":
/usr/lib/python3/dist-packages/cubical/database/pickled_db.py:163: SyntaxWarning: "is" with a literal. Did you mean "=="?
  assert (self.mode is "load")
/usr/lib/python3/dist-packages/cubical/machines/complex_2x2_machine.py:190: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if self._estimate_pzd and self._pzd is 0:
Traceback (most recent call last):
  File "/usr/lib/python3.8/py_compile.py", line 144, in compile
    code = loader.source_to_code(source_bytes, dfile or file,
  File "<frozen importlib._bootstrap_external>", line 846, in source_to_code
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/lib/python3/dist-packages/cubical/plots/gainsols.py", line 179
    ("r", re1), ("b", im1),
              ^
SyntaxError: invalid syntax

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.8/py_compile.py", line 197, in main
    compile(filename, doraise=True)
  File "/usr/lib/python3.8/py_compile.py", line 150, in compile
    raise py_exc
__main__.PyCompileError:   File "/usr/lib/python3/dist-packages/cubical/plots/gainsols.py", line 179
    ("r", re1), ("b", im1),
              ^
SyntaxError: invalid syntax

https://github.com/ratt-ru/CubiCal/blame/master/cubical/plots/gainsols.py#L179

in the interest of producing a working Debian package it is important the code base is in py3 syntax compatible.

[JSKenyon]

Pinging @o-smirnov - I tried to figure this out but I don't have experience with the plotting functionality. I also encountered a missing dependency when invoking plot-gain-solutions as ipdb is not installed by default. I would strongly encourage removing debugging code.

[gijzelaerr]

index 0573f33..5863ef4 100644
--- a/cubical/plots/gainsols.py
+++ b/cubical/plots/gainsols.py
@@ -176,8 +176,8 @@ def plot_bandpass(sols, plot_diag='ap', plot_offdiag='', gaintype=("Bandpass", "
             ax.plot(freq, x2[ts].imag, '+y', ms=2, label=im2 if not ts else None)
         if legend:
             ax.legend(handles=[mpatches.Patch(color=col, label=label) for col, label in
-                                   ("r", re1), ("b", im1),
-                                   ("c", re2), ("y", im2)
+                                   [("r", re1), ("b", im1),
+                                   ("c", re2), ("y", im2)]
                                ], loc="upper center", ncol=2, fontsize=options.font_size)

     def _make_ap_plot(ax, ax2, freq, x1, x2, corrs, legend):
@@ -191,8 +191,8 @@ def plot_bandpass(sols, plot_diag='ap', plot_offdiag='', gaintype=("Bandpass", "
             ax.plot(freq, abs(x2[ts]), '+b', ms=2, label=amp2 if not ts else None)
         if legend:
             ax.legend(handles=[mpatches.Patch(color=col, label=label) for col, label in
-                                   ("r", amp1), ("b", amp2),
-                                   ("c", ph1),  ("y", ph2)
+                                   [("r", amp1), ("b", amp2),
+                                   ("c", ph1),  ("y", ph2)]
                                ], loc="upper center", ncol=2, fontsize=options.font_size)

     for iant, (ant, (time, freq, g00, g01, g10, g11)) in enumerate(sols.items()[:max_sols]):
@@ -300,7 +300,7 @@ def plot_gain(sols, plot_diag='ap', plot_offdiag='', gaintype=("Gain", "Offdiag
             ax.plot(time, x2[:, fs], '+b', ms=2, label=corrs[1] if not fs else None)
         if legend:
             ax.legend(handles=[mpatches.Patch(color=col, label=label) for col, label in
-                                   ("r", corrs[0]), ("b", corrs[1]),
+                                   [("r", corrs[0]), ("b", corrs[1])]
                                ], loc="upper center", ncol=2, fontsize=options.font_size)

     def _make_reim_plot(ax, time, x1, x2, corrs, legend):
@@ -313,8 +313,8 @@ def plot_gain(sols, plot_diag='ap', plot_offdiag='', gaintype=("Gain", "Offdiag
             ax.plot(time, x2[:, fs].imag, '+y', ms=2, label=im2 if not fs else None)
         if legend:
             ax.legend(handles=[mpatches.Patch(color=col, label=label) for col, label in
-                                   ("r", re1), ("b", im1),
-                                   ("c", re2), ("y", im2)
+                                   [("r", re1), ("b", im1),
+                                   ("c", re2), ("y", im2)]
                                ], loc="upper center", ncol=2, fontsize=options.font_size)

     def _make_ap_plot(ax, AX2, time, x1, x2, corrs, legend):
@@ -328,8 +328,8 @@ def plot_gain(sols, plot_diag='ap', plot_offdiag='', gaintype=("Gain", "Offdiag
             ax.plot(time, abs(x2[:, fs]), '+b', ms=2, label=amp2 if not fs else None)
         if legend:
             ax.legend(handles=[mpatches.Patch(color=col, label=label) for col, label in
-                                   ("r", amp1), ("b", amp2),
-                                   ("c", ph1),  ("y", ph2)
+                                   [("r", amp1), ("b", amp2),
+                                   ("c", ph1),  ("y", ph2)]
                                ], loc="upper center", ncol=2, fontsize=options.font_size)

[o-smirnov]

Well yes exactly it just needed [] around the list of tuples. @gijzelaerr can you commit that, or is my distinguished professorial input required?

[JSKenyon]

Ok - I decided to fiddle. Will put up a PR now but it needs your involvement Oleg as there is lots of madness under Python3.

[gijzelaerr]

I wanted to make a list of required changes, but when this patch is applied at least the resulting Debian package installs. The SyntaxWarning is still there though, that requires a bit more domain-specific knowledge.

I recommend you guys set up a mypy and pycodestyle check on Travis to prevent check-in of non-functioning code.

[o-smirnov]

I do run it with py3 routinely now, though perhaps that code path hasn't been touched. @JSKenyon do you understand the syntax warning>

[JSKenyon]

I will prod at the syntax error in the morning! Looks like it just wants comparison to be done using a logical operator rather than a check that they are the same object.

On 06 May 2020, at 17:04, Oleg Smirnov notifications@github.com wrote:

 I do run it with py3 routinely now, though perhaps that code path hasn't been touched. @JSKenyon do you understand the syntax warning>

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[JSKenyon]

Confirmed - doing a is 1 is not correct. Easily fixed with a == 1.

[gijzelaerr]

one should never use is for value equality, only reference equality. You run into weird bugs otherwise:

In [1]: one = 1                   

In [2]: a_lot = 20000             

In [3]: one is 1                  
Out[3]: True

In [4]: a_lot is 20000            
Out[4]: False

[JSKenyon]

@gijzelaerr Just letting the tests finish up - will merge and do a release once they have.

[o-smirnov]

one should never use is for value equality, only reference equality. You run into weird bugs otherwise:

I knew that. Only for None. Did some "ises" of something else leak in?

[JSKenyon]

Yeah - for the special --model-list 1 case. You were checking for model_source is 1 rather than model_source == 1.

[o-smirnov]

Ah ok. I blame senility.

[JSKenyon]

@gijzelaerr v1.5.2 release has the necessary fixes. Please close this if it solves your problems.

[JSKenyon]

Closing for now - reopen if necessary @gijzelaerr.