Closed martinbra closed 4 years ago
Oh, I forgot to update the version number.
@chubin Thanks for your review and valuable tips. I also think that we should try to keep the original spirit alive.
I reviewed the points we mentioned and would appreciate if you could have another look.
I also wonder what your thoughts are on the --hemisphere
short switch argument -s
(from sphere, since -h
is taken for the help command). Do you think it's ok? Or should we use -e
or another letter?
I've plotted a full month for every moon size and hemisphere, and I didn't find any inconsistencies. out2.txt - 427KB
It looks good to me. Great work, Martin! I would merge it and then let's make the next step: enable it in wttr.in
Fixes issue #1. If closed, we can create a issue for the still missing hemisphere autodetection.
I had to fix moons lines lenght for the unequal length lines ending in
\ "
in order to correctly read the moon upside down.I tried to keep the most of the code unchanged, and kept that long
if/elif
to getchar = background_nlines[lin][col]
. Was it in the original code? Personally, I would change that to a dictionary to remove the if/elif and simplify it tochar = backgrounds[nlines][lin][col] if nlines in backgrounds.keys() else '@'
If you like the idea, let me know and I can implement it.Follows all the moons (first original and then upside down) for comparison and finally a waxing moon to show the change in direction: