LCOGT / neoexchange

NEO observing portal
GNU General Public License v3.0
7 stars 1 forks source link

GIF movies now have excess whitespace around image #610

Closed talister closed 2 years ago

talister commented 2 years ago

With the change in https://github.com/LCOGT/neoexchange/commit/ddaee0047354617da0caff03d8077603fe077d8d on feature/lightcurve_converter to make the GIF movies work on Matplotlib > 3.4, we now have an excess amount of whitespace at the top of movies compared to the older version (matplotlib<3.3) that is pinned on main. Screen shots of "old" (from 20220721/65803_2947583_framemovie.gif) and "new" (from 20221002/65803_3015270_framemovie.gif) attached

gifmovie_old gifmovie_new

jchate6 commented 2 years ago

Look at that tail...

talister commented 2 years ago

Look at that tail...

While I am not sure I should be encouraging this, there are deeper multicolor tail data in https://observe.lco.global/requests/3015287 when I put in a gri sequence to see if the scheduler was working when it was looking empty yesterday afternoon

jchate6 commented 2 years ago

How does this look? 2022AB_2762138_framemovie

I'm having issues getting pyslalib.slalib working so I can't do a proper test with lightcurve extraction, and not having it makes all of the

try:
    import pyslalib.slalib as S
except:
    pass

statements deceptively successful.

jchate6 commented 2 years ago

The method that sup_titles get applied and default axes scaling was changed. The size is a little different, but I don't think it's bad. I can make it bigger if you like.

jchate6 commented 2 years ago

I also learned about the very interesting astropy.Time package...

jchate6 commented 2 years ago

Update: the gca(projection=wcs) that applies the moving grid is also breaking in matplotlib 3.4.0.

Looking for a work around.

talister commented 2 years ago

Update: the gca(projection=wcs) that applies the moving grid is also breaking in matplotlib 3.4.0.

Looking for a work around.

I fixed that in this commit: https://github.com/LCOGT/neoexchange/commit/ddaee0047354617da0caff03d8077603fe077d8d

jchate6 commented 2 years ago

oh neat.

talister commented 2 years ago

The method that sup_titles get applied and default axes scaling was changed. The size is a little different, but I don't think it's bad. I can make it bigger if you like.

That looks a lot better. What would help the most for the Didymos observations would if the secondary title include the frame number (the 0xyz out of the middle of the filename) as well. When clipping the output photometry, I only have a filename and JD available so matching with the frame in the animation is harder...

talister commented 2 years ago

oh neat.

They did a bit of a dirty on us... If you remove the over-zealous warnings-ignoring in our code, it says what we were doing was deprecated in 3.4 and would fail after 2 minor releases. We're on 3.5.x something so within that, unless they meant 3.4.1 and 3.4.2 and then break...

jchate6 commented 2 years ago

I was not in the right branch for that test. Results: 2022AB_2762138_framemovie

Could you make the frame reference an "always 4 digit, always zero prefixed" number (forget the right f-string needed) so it's (a little) more distinct from the "(1 of 8)" video frame </pedant>

jchate6 commented 2 years ago

easy: 2022AB_2762138_framemovie

jchate6 commented 2 years ago

Slightly different format. Maybe cleaner? 2022AB_2762138_framemovie

talister commented 2 years ago

Like the frame number, don't like the non-ISO 8601 date, needs to be YYYY/MM/DD HH:MM:SS as 24hrs (unless that AM is something other than 'am and 'pm')

jchate6 commented 2 years ago

Oh that's interesting. That code is the same... 2022AB_2762138_framemovie

talister commented 2 years ago

Oh that's interesting. That code is the same... 2022AB_2762138_framemovie 2022AB_2762138_framemovie

Why does stuff silently change in the background when you don't alter the code... !? sigh

jchate6 commented 2 years ago

I know the %x and %X uses assumptions about local time formats, but I'm not sure why that would be different here.

talister commented 2 years ago

I know the %x and %X uses assumptions about local time formats, but I'm not sure why that would be different here.

Probably server doesn't have a LOCALE set (or a generic one) and locally it has "America" or "Pacific/Los Angeles" timezone or some such . I think explicit setting of the format with a format string that can be re-used/altered in 1 place in the code would be best.

jchate6 commented 2 years ago

better to just be explicit.