Clarify style of attribute comments in the Design Guide.

[tannewt]

And update the core attributes to match the style.

@adafruit/circuitpythonlibrarians Please check this out too.

[deshipu]

By the way, we could use this extension for Sphinx: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/

[tannewt]

Would you want to switch to google style docstrings then @deshipu ?

[arturo182]

Celsius

[arturo182]

Celsius

[dhalbert]

writable

[dhalbert]

Napoleon looks really nice. I don't know how it would be in practice but it looks a lot less finicky than RST. It might really encourage folks to write more documentation.

[tdicola]

Here's my feedback on the whole file (it's easier to put this all in one spot):

• 'Cookiecutter is a cool tool...' - I'd refrain from using 'cool' to describe tooling in technical docs like this, it can come across with the wrong tone (i.e. you aren't 'cool' if you aren't using cookiecutter). Better to take a neutral tone and remove it, i.e. 'Cookiecutter is a tool'.
• Scope, it should clarify what this covers. The top says Circuitpython and its libraries--is that just stuff in the circuitpython repo and the internal modules? Third party modules made by Adafruit? Third party modules made by anyone else? Modules included in the bundle, community bundle, etc? Good to be explicit here.
• Module naming - This is a good spot to also describe modules with underscores / spaces (like Adafruit_Thermal_Printer). There's confusion with how they work in the cookiecutter template and dashes vs. underscores in the name, travis confis, RTD URLs, etc. A canonical reference for this here would be great.
• Verify your device - Shouldn't failure to find the device raise a RuntimeError instead of ValueError? The Python docs intend for ValueError to be used when a parameter to a function has an incorrect value (https://docs.python.org/3/library/exceptions.html#ValueError), like it's outside the range of allowed values. Failure to find a device typically indicates some problem talking to the hardware, like wiring or assembly issues. It seems like RuntimeError fits that scenario the best.
• Getters / setters - I think there needs to be some discussion of how properties that require multiple values are handled. For example setting the tuning frequency of an FM transmitter requires both a frequency and antenna capacitance to be specified. This state doesn't map cleanly to python's concept of a property (single value) and there are a lot of ways to handle it, like explicit get/set functions, using tuples (but then have memory allocations and conflicts with the later advice to avoid allocations), etc. Concrete examples and advice here would be beneficial.
• Design for compatibility with CPython - Good to mention and solidify any conventions here, like the current philosophy of appending u to Micro/CircuitPython-specific versions of extended Python libraries (i.e. ustruct, utime, etc. that add extra things or differ from cpython versions).
• Attributes - I think this should add some advice on how to document attributes that aren't python properties. For example instance variables added in the init (like a self.timeout_s) or decorator classes (the adafruit_register instances) which don't have docstrings. Is there perhaps a way to force Sphinx to document these attributes? Some advice here would be super helpful I think.
• Sensor properties and units - I'd add to this any conventions codified in circuitpython today for unitless values too, like ADC/DAC/PWM duty cycles values being 16-bit unsigned numbers (regardless of underlying hardware resolution).
• Common advice - This is a little ambiguous and hard to understand what the advice is right now. IMHO I'd remove it or add some concrete examples to help people understand what common functions, etc. to share between drivers. For example gain is typically a property of all light sensors, or data_rate / sample_rate is common to many sensors.

Some stuff to add:

• I'd mention how to run sphinx and verify docs don't have any warnings, etc. This will help driver writes validate their docs before pushing up to travis, etc.

[tdicola]

And yeah totally agree +1 on napoleon or NumPy / Google Python Style / etc. doc strings vs. RST. The RST ones are hard to read in the raw form once you get more than a few parameters. Not the end of the world to live with them, but maybe explicit advice to move to napoleon style for new docs would be good to think about and put in.

[deshipu]

Would you want to switch to google style docstrings then @deshipu?

Personally I don't have a preference — I'm used to all three styles of docs, and they are more or less equivalent in my eyes. I also don't really have time to go over our documentation and fix it in all the places. I just thought that I will point out that we have some choice in this matter.

[tannewt]

@arturo182 and @dhalbert Done.

Regarding Napoleon and Google style comments: I've opened #607 to discuss it. The core APIs have to be rST because they aren't pull from docstrings.

@tdicola Great feedback! Here is a bullet-for-bullet reply:

• Cookiecutter - Removed "cool".
• Scope - Done, including links!
• Module naming - Added
• Verify your device - Agreed, changed.
• getters / setters - I don't feel like I've seen this enough to provide general guidelines. Lets talk specifics on cases you've seen. Please file issues on the repos where this occurs and assign them to me.
• Design for compatibility with CPython - Done
• Attributes - I added specific guidelines for comment location with this PR. Please open a separate issue about enforcing that they exist. Thanks!
• Sensor properties and units - Added duty_cycle, frequency and value.
• Common advice - Agreed, removed.
• How to run sphinx - I don't think this fits this doc because its about design patterns and philosophy. Its documented here but should also be added to a learn guide.

Please take another look everyone. Thanks!

[dhalbert]

OK by me.

[kattni]

Misspelled word, question about same line.

[kattni]

This should be it's.

However, is there a reason to include "even though it's weird?" Seems like it should be enough to state "Comment is after".

[tannewt]

Tweaked the data descriptor description. Please take another look @kattni

[kattni]

Looks great! Thanks for the update!

[tannewt]

Look good to you @tdicola?

[tuupola]

Not directly relate to this pr, but is it intentional that all other properties are lowercase except eCO2 and TVOC? It would make sense to keep all properties lowercase.

[ladyada]

TVOC and eCO2 are not words but acronyms ... is my thinking. anyone want it lowercased? I could shim existing libraries if people feel strongly!

[tannewt]

I'd probably write out what they stand for in lowercase if we were to change it. They'd then be long but explanatory. @tuupola its a good idea. Mind opening an issue for it?

[tuupola]

In code I find consistency a good thing. If everything is lowercased there is no need to remember whether one should use capital letters or not. For example lux might be confusing. Is it acronym or word? Should it be sensor.lux or sensor.LUX? Also when seeing sensor.TVOC one is not really sure if it is a constant or property.

I do not have strong feelings about this, but my choice would be lowercased acronyms sensor.tvoc and sensor.eco2.

[ladyada]

ill defer to @tannewt on this one, i don't care too much, we'll just have to update our existing libraries :)

[kattni]

Thanks for the great work!

[tannewt]

@tdicola oked the merge at lunch this week.