Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create growingrecs.py #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Create growingrecs.py #1

wants to merge 1 commit into from

Conversation

fhocutt
Copy link

@fhocutt fhocutt commented Aug 30, 2014

Command-line tool to find the most common planting conditions for a given crop. This version counts combinations of planted_from and sunniness, but as planted_from is often left blank this may not be the best choice.

Command-line tool to find the most common planting conditions for a given crop. This version counts combinations of planted_from and sunniness, but as planted_from is often left blank this may not be the best choice.
@@ -0,0 +1,100 @@
#!/usr/lib/python2.7

# MIT License.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BSD license was what I chose for the github repo. It might be good for us to develop some suitable boilerplate to go at the top of each script, with links to the github, license, and other info. Not urgent, just a thought.

@Skud
Copy link

Skud commented Aug 30, 2014

This is a great start :)

I'll leave some line-by-line comments but also a few general ones.

Firstly, I think we should create subdirs, v0 and v1, for different API versions, and put this (along with other scripts with this API) in the v0 directory.

Secondly, I think it would be useful to name scripts starting with, say, the main object in the database we're dealing with. It might make it easier for people to find examples of what they're looking for. So this for example would be crop_growing_recs.py or maybe crop_growing_advice.py ("growing advice" being the term we more commonly use in the project).

You asked me to comment on the level of internal documentation/comments and I'll do so inline, but overall it looks pretty good and and the code+comments together are quite clear to me -- and I don't particularly speak Python :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants