nsbruce commented 4 years ago

I'm not sure if you're accepting PRs. This PR is not complete yet but I thought I would try it out and see if you're ok with me issuing them.

Fixed in this PR:

Call to FrameTimestamp.jd -> FrameTimestamp.mjd since jd doesn't exist.
A datetime setter was expecting a unix timestamp and receiving a FrameTimestamp.
The sort override methdods in fitsidi.py's _UVData class self referenced the class with the class name (syntax that I believe should work or maybe just did in previous python versions). Explicitly self referencing with self.__class__ fixed an error of _UVData is not defined.

Still needs work before PR is merged:

When I check the output of correlateTBN (*.FITS_1) with the imageIDI.py script I can't see anything (see image below). Now I'm trying to see whether this is an error with correlateTBN or with imageIDI.

I'll look at this again later and update this PR unless someone else gets there before me.

coveralls commented 4 years ago

Coverage remained the same at 82.957% when pulling 6f7631957b36056025ffb87df458aa88085d96fe on nsbruce:correlateTBN-fixes into 4b64530d40dc263b12269bd2593ff7fd6f24685b on lwa-project:master.

codecov[bot] commented 4 years ago

Codecov Report

Merging #8 into master will not change coverage. The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master       #8   +/-   ##
=======================================
  Coverage   83.06%   83.06%           
=======================================
  Files          65       65           
  Lines       21134    21134           
=======================================
  Hits        17555    17555           
  Misses       3579     3579

Impacted Files	Coverage Δ
lsl/writer/fitsidi.py	`84.21% <0.00%> (ø)`

jaycedowell commented 4 years ago

Sorry about all this. Many of these problems were related to bugs in LSL and the scripts that we fixed locally for the NRAO Synthesis Imaging Workshop but never pulled back into the repo. You might try to latest version of master to see if all of your problems are addressed there. It's also probably a good idea to stick with the released versions rather than installing master (although that really wouldn't have helped here).

nsbruce commented 4 years ago

This is great. Master is working well. Thanks for all of the quick fixes @jaycedowell!

lwa-project / lsl

correlateTBN.py bug fixes #8

Fixed in this PR:

Still needs work before PR is merged:

Codecov Report