initial PR for sun angle input

[tomellm]

#5 Created a second cli function that allows the user to input the sun altitude directly. This avoids having to calculate the shadows completely and just does a sun angle comparison directly.

Doing a quick comparison of the outputs using find and find_sun it seems that they are the same, but there is the obvious difference of how blurred the sun version is. I'm not quite sure as to why that is.

*Will edit notebook once feature is more stable

Please consider that I have practically no knowledge on python best practices so please do correct me on where things should be done differently.

[tomellm]

@GalenReich

[GalenReich]

Hey! Looks good for a first pass! I've added a few comments, the main thing is about making sure the Finder object can be reused multiple times without getting into a state that breaks. Do ask any questions if anything is unclear.

Formatting with black

I ran the black formatter over the code - it's a handy way of making sure that your Python formatting follows the standard.

I realize that in doing this review the docs for developers are nonexistant in this repo - apologies, that's on me!

You can use poetry install --with dev to get black, and then you can invoke it with black ./ in the project folder and it will format all the Python files to keep things clean and tidy 🧹

[GalenReich]

Hi @tomellm, just looking through this now! Thanks! 🙌

As a hopefully helpful note, it's not good practice to force push when you're working on any code with another person (as with a pull request under review). Force pushing rewrites the history of the branch and makes it harder to collaborate and follow the code development. For example, force pushing overwrote my formatting commit - this isn't a problem in this instance, but I wanted to let you know for future reference.

A couple of tips:

Use git pull often when collaborating, it means that your local code won't get behind the code on the origin, and help you avoid merge conflicts.

If you do have to force push, try using git push --force-with-lease it will mean you don't accidentally overwrite commits your local doesn't know about.

Hope that helps - I only found out about --force-with-lease fairly recently and my mind was blown 🤯

[tomellm]

Hey @GalenReich, sorry for using force push. In all the environments I have been in we have always practiced the rebase and force push approach and avoided committing to someone else's branch unless discussed. But in any case I will stop doing this in the future and thank you for telling me. It makes sense that in a more collaborative environment people would push to each-others branches. ;)

[GalenReich]

Yeah, my understanding (though I'm happy to be corrected) is that once a PR is opened, committing to the branch is fair game as part of the PR process. And adding more commits once a review is complete just helps track the changes - it'll all get squashed (or merge commited) at the end of the PR anyway, so consolidating commits as you go isn't needed.

But it's all a learning journey, and is ultimately about what works! I'm enjoying reviewing this PR and hope it's interesting and helpful for you! 🚀

[tomellm]

Yess its definitely very helpful, I'm very happy that you are having so much patience with me ;). As for the commiting, it totally makes sense that once a PR is opened its fair game, I'm just used to a different style I guess. (And the squashing does tackle the consolidation issue).

I would love to work on another issue on this project (or another project) if thats welcome. Also feel free to merge this one if the last changes work for you.

[GalenReich]

Okay, this is looking great! Thank you so much for your hard work. I'm going to do some testing now myself, and amend the accompanying notebook so it includes the new feature! Then I'll get it all merged in one go 🌟