afeinstein20 / eleanor

A tool for light curve extraction from the TESS FFIs.
MIT License
92 stars 39 forks source link

multisector() returns TESS magnitude of 999 #231

Closed GijsMulders closed 3 years ago

GijsMulders commented 3 years ago

The multisector() command returns a value of 999 for the TESS magnitude in each sector, is this intended?

Example: stars= eleanor.multisector(tic=tic, sectors = 'all') print (stars[0].tess_mag) gives 999 while the same command with Source() gives the actual magnitude star= eleanor.Source(tic=tic) print (star.tess_mag)

benmontet commented 3 years ago

Hm, that shouldn't happen! The 999 is a (poorly documented) hack that gets input for the cases whenever a target isn't in the TIC and doesn't have a calculated tess_mag. But given that you're calling multi_sector using the TIC ID directly that should never be the case...is this happening for you for one star or all stars? Can you give an example TIC where you have this behavior? multi_sectors is just looping over Source so it must be some silly edge case!

Edit: Can you also check what stars[0].tic_version is in your example case?

GijsMulders commented 3 years ago

The last star I tried this on was TIC 313935081 (but it seems to happen for every star I tried so far)

stars[0].tic_version is set to None

(and 20190415 when I use .Source()

GijsMulders commented 3 years ago

I looked into the source and it seems that multisector calls: coords, _, _, _ = coords_from_tic(tic) which doesn't store the tess_mag. and then Source() is called with the coords keyword which skips the tess_mag calculation using coords_from_tic and sets it to 999 instead.

benmontet commented 3 years ago

Ah yes that's definitely it! And it explains why I didn't run into it in my (single) test, because that bit is inside an if sectors=='all' statement to get the list of sectors from tess-point and I tested passing through a list of sectors so my setup was different.

Seems straightforward enough to sort but I'm about to head to bed and am not confident I wouldn't mess it up further if I did it right now; watch this space for an update tomorrow!

benmontet commented 3 years ago

Okay try now! The just-pushed github version should have the fix in it, which will not be in pypi at present but will be filtered through to it at the next version upgrade.

I think the fix shouldn't mess up anything else (in particular running things locally) so we should be good to go but let me know if this does/does not work for you!

GijsMulders commented 3 years ago

Tested and works!

Thanks for the quick fix :)

benmontet commented 3 years ago

Great! Closing for now; if you (or any future readers) find this has caused some other issue downstream feel free to re-open