feat(event): add support for Earth seasons

[LiamNgn]

Q	A
Bug fix?	no
New feature?	yes
Related issues	Fix #23
Has BC-break	no
License	CeCILL-C

[Deuchnord]

Thank you for your contribution!
Just made some change requests to enhance the general code maintainability. If you need help or have some remarks, feel free to ask :)

[LiamNgn]

I have recently updated my code based on your feedback. If you find any change needed, just tell me. Thank you.

[Deuchnord]

Thanks, we are on a good path!
Just making a few new comments :)

[Deuchnord]

Didn't see it immediately, but since we are adding a new feature, your branch should be based on features, not on main. Can you rebase it?

[LiamNgn]

I have updated the code according to your suggestions. Regarding the Season Change, I have created a new enum class for these. Is that ok? Moreover, I'm unsure how to do rebase so I just edit the base branch on these issue. Is that correct? If not how can I rebase from my forked repository? Thank you.

[Deuchnord]

Moreover, I'm unsure how to do rebase so I just edit the base branch on these issue. Is that correct? If not how can I rebase from my forked repository? Thank you.

It's actually not sufficient. Rebasing on features means that your branch's base has to be moved from main to features.

This is what we currently have:

                ------------------------------- features
              /
-------------o---------o--------------------- main
                        \
                         ---------------------- dragontunglam:LiamNg

And this is what we should have:

                                     ---------------------- dragontunglam:LiamNg
                                   /
                ------------------o------------- features
              /
-------------o---------o--------------------- main
                         

The goal of this is to ensure that new features come in x.y.0 releases, whereas bugfixes are done in the main branch directly and come in x.y.z (with z > 0) releases.

There are many ways to achieve this, but in your case, I think the simplest way would be to rename your branch to a temporary name, recreate a new one with the same name of your current branch but based on features and cherry-pick your commits to the new branch.

You should do these actions before making any new change to your work to ensure nothing is lost.

# Ensure you have the last version of the branches:
git remote add upstream https://github.com/kosmorro/lib.git
git fetch --all --prune

# Rename your branch:
git branch -m LiamNg tmp

# Switch to `features` branch, update it and recreate your branch:
git checkout features
git pull upstream features
git checkout -b LiamNg

# Cherry-pick your commits:
git cherry-pick 9d7240bb4ab54d0e13e81f5875e575c340129b4d
git cherry-pick 3704175c98c9453637f4d865bb51128df87c3842
git cherry-pick 95cbfc970dc73fbefdca0ae8e636ddf1254070cb
git cherry-pick 0425a3020446edc0f8ab2502c2f46255a6667275

# Force-push your new branch to update the PR:
git push -f origin LiamNg

Then, check that all your work (and only yours) is well on your PR, and that there are only your commits and finally, delete your temp branch (but only if all is good):

git branch -D tmp

Regarding the Season Change, I have created a new enum class for these. Is that ok?

This is definitely a very good idea! But the event_type field of Event should remain an EvenType (otherwise, it would introduce too much complexity for the end developers).
We then could reintroduce a SEASON_CHANGE in the EventType and put the new SeasonType in the details field of the Event class, which would become a dictionnary (it is currently a string).

So the actions would be:

1. In the EventType, add an SEASON_CHANGE constant
2. In the model.py module, line 157, change the type of the details parameter from str to dict
3. In the event.py module, line 186, change the line to:

   details={'deg': elongation}

   (Bonus: this makes it much easier for the developers to use this data! 😄
4. On your events.append(...) instruction, change the first parameter of the Event object to EventType.SEASON_CHANGE and add a details={'season': season}, where the season variable will be one of the constants in the SeasonType.

Note that this will be a breaking change, so let's make profit that we're still not v1.0 to do it! 😄

[Deuchnord]

That looks very good, just a little change and we'll be good!
Can you just check that the tests in the documentation are up-to-date? Note that you can run them by invoking make doctests.

[coveralls]

Coverage increased (+1.3%) to 82.959% when pulling d08e21c on LiamNgn:LiamNg into 2884da5 on Kosmorro:features.

[Deuchnord]

This looks very good!
You only need to fix the legacy tests (they broke because we changed the details in the Event) and to fix Black's alerts (you can fix them quickly with make black), and we'll be good to merge! 😄

Btw, the Commit / Message validation job on the CI can be safely ignored, it is a legacy test I have recently removed.

[Deuchnord]

Just made some very minor fixes to enhance the code quality.
Thank you for your contribution, it will be merged as soon as the CI passes! 😄