Some corrections that should be made in "Drawing a finder chart for Comet NEOWISE"

[reza-ghazi]

Issues:
line 31: comets = comets.set_index('designation', drop=False)

should change to:

comets.set_index('designation', drop=False, inplace=True)

In line 33: comet = sun + mpc.comet_orbit(row, ts, GM_SUN)
should change to:
comet = sun + mpc.comet_orbit(row.iloc[0], ts, GM_SUN)
or
comet = sun + mpc.comet_orbit(row.iloc[1], ts, GM_SUN)

In line 105: t_comet.utc_strftime('%-m/%d')

string format should change to %m/%d

And also:
If we want to get an exact output as shown in the picture of the example, the following changes should be made too:

In line 75: marker_size = (0.5 + limiting_magnitude - magnitude) ** 2.0

2.0 should change to 2.5

In line 87: fig, ax = plt.subplots(figsize=[9, 9])
figsize should be [13, 13]

Thank you,

[brandon-rhodes]

Good morning! To help me understand the motivation behind each change, could you list a rationale, or error message (if the script is not working on your system), behind each one? I just re-ran the script locally and confirmed that the output here on my Linux laptop is pixel-for-pixel and byte-for-byte identical to the image included in the documentation, so we are going to need to figure out step by step why you are getting different results. Thanks!

[reza-ghazi]

Good morning Brandon,
I am using Windows 10, with Anaconda Python 3.8.5 (latest version in anaconda env so far).

The first error that I have got is as follow:

ValueError                                Traceback (most recent call last)
<ipython-input-1-538201e9da4b> in <module>
     31 comets = comets.set_index('designation', drop=False)
     32 row = comets.loc['C/2020 F3 (NEOWISE)']
---> 33 comet = sun + mpc.comet_orbit(row, ts, GM_SUN)
     34 
     35 # The Hipparcos mission provides our star catalog.

D:\anaconda3\lib\site-packages\skyfield\data\mpc.py in comet_orbit(row, ts, gm_km3_s2)
    205 def comet_orbit(row, ts, gm_km3_s2):
    206     e = row.eccentricity
--> 207     if e == 1.0:
    208         p = row.perihelion_distance_au * 2.0
    209     else:

D:\anaconda3\lib\site-packages\pandas\core\generic.py in __nonzero__(self)
   1327 
   1328     def __nonzero__(self):
-> 1329         raise ValueError(
   1330             f"The truth value of a {type(self).__name__} is ambiguous. "
   1331             "Use a.empty, a.bool(), a.item(), a.any() or a.all()."

ValueError: The truth value of a Series is ambiguous. Use a.empty, a.bool(), a.item(), a.any() or a.all().

After investigating and browsing comet data, I saw two rows of data in the Comet data frame for 'C/2020 F3 (NEOWISE)'. So, for the correct answer, we have to choose one of them, and with a little bit of play around, I found both are giving the same result.

Then I got the second error:

ValueError                                Traceback (most recent call last)
<ipython-input-2-7892e3f069a8> in <module>
    103 ax.plot(comet_x, comet_y, '+', c=comet_color, zorder=3)
    104 
--> 105 for xi, yi, tstr in zip(comet_x, comet_y, t_comet.utc_strftime('%-m/%d')):
    106     text = ax.text(xi + offset, yi - offset, tstr, color=comet_color,
    107                    ha='left', va='top', fontsize=9, weight='bold', zorder=-1)

D:\anaconda3\lib\site-packages\skyfield\timelib.py in utc_strftime(self, format)
    536         """
    537         whole, fraction, is_leap_second = self._utc_float(0.0)
--> 538         return self.ts._strftime(format, self.whole, fraction, is_leap_second)
    539 
    540     def _utc_tuple(self, offset=0.0):

D:\anaconda3\lib\site-packages\skyfield\timelib.py in _strftime(self, format, jd, fraction, seconds_bump)
    210             tup = year, month, day, hour, minute, second, weekday, yday, z
    211             if getattr(jd, 'ndim', 0):
--> 212                 return [strftime(format, item) for item in zip(*tup)]
    213             return strftime(format, tup)
    214 

D:\anaconda3\lib\site-packages\skyfield\timelib.py in <listcomp>(.0)
    210             tup = year, month, day, hour, minute, second, weekday, yday, z
    211             if getattr(jd, 'ndim', 0):
--> 212                 return [strftime(format, item) for item in zip(*tup)]
    213             return strftime(format, tup)
    214 

ValueError: Invalid format string

Then I changed the format string to '%m/%d,' which is correct. I believe it is a typo.
**
For the extensive data in my experience, using inplace=True is more efficient than replacing the data frame again after changing the index column. Both are working well the way you promoted and the way that I suggested, and it is just a suggestion.

**
After I get the result, I saw the figure and stars sizes do not match the picture you put on the website with my monitor resolution; then, I suggested the new sizes.

That is all.

Thank you for your response and fantastic work in Skyfield, which is my favourite tool for my works.

[brandon-rhodes]

The first error that I have got is as follow: …

ValueError: The truth value of a Series is ambiguous. Use a.empty, a.bool(), a.item(), a.any() or a.all().

There are two Comet “C/2020 F3 (NEOWISE)” in the same file?! What? 🙂

Thanks for pointing that out, the older CometEls.txt file on my laptop didn’t have that problem (dated July 12). I had to remove it and let Skyfield download a fresh copy to see the error.

If a comet can appear twice in the file then (a) I will need to rename row to something plural like matching_rows or matches, and then suggest how users might choose from between the two rows. Should they, for example, choose the row from the most recent circular — which it looks like will be the “reference” that comes last in the alphabet? In that case, I should suggest a pre-processing step that removes duplicates in favor of the most recent MPC number.

I'll see about making such a change.

Then I got the second error: …

ValueError: Invalid format string

Well, drat. I guess that’s what https://strftime.org/ means when it says “Platform specific” for:

%-m | Month as a decimal number. (Platform specific) | 9

I had hoped that all modern platforms would support %-m but, if Windows does not, then I'll instead run tstr = tstr.lstrip('0') on the string once it’s built, to make it compatible with as many systems as possible.

For the extensive data in my experience, using inplace=True is more efficient than replacing the data frame again after changing the index column. Both are working well the way you promoted and the way that I suggested, and it is just a suggestion.

I do see a speedup from 0.45 ms to 0.30 ms here on my laptop when making that change. But given the controversy surrounding inplace= in Pandas, I generally avoid it in code that might be copied by folks who are new to Pandas and don’t know the best practices yet.

• API/DEPR: Deprecate inplace parameter pandas-dev/pandas#16529
• https://stackoverflow.com/a/60020384/85360

So for now I suspect that I'll do the assignment statement, just to keep the Pandas patterns here as simple as possible for new users. But thanks for pointing out the performance difference!

After I get the result, I saw the figure and stars sizes do not match the picture you put on the website with my monitor resolution; then, I suggested the new sizes.

I guess I'll have to learn sometime how matplotlib decides on DPI when rendering. I had hoped it would ignore the system and always do the same thing, given the same Python code. Alas!

Thank you for your response and fantastic work in Skyfield, which is my favourite tool for my works.

Thanks very much for the kind words! I'm glad you are finding Skyfield useful, it is very encouraging to create something and then have it be useful to people in real work.

[brandon-rhodes]

There, hopefully those improvements prevent future users from running into these problems. Thanks again!

[reza-ghazi]

That's awesome. Thank you again.