Skip to content

docs/library/neopixel.rst: Update neopixel docs to hide dunders. #16764

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

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

Conversation

tobes
Copy link

@tobes tobes commented Feb 16, 2025

Summary

Change documentation to show using len(np) to access number of pixels rather than NeoPixel.__len__(). Also use update for setting and getting items via np[index].

I was looking at the NeoPixel documentation and saw that it was showing dunder methods for accessing number of pixels and also for setting/reading pixels ie NeoPixel.__len__(), NeoPixel.__setitem__(), NeoPixel.__getitem__() rather than using len(np), np[index].

It is good practice especially for beginners that we should be showing correct access of these items rather than encouraging people to use private methods.

I used the python documentation https://docs.python.org/3/library/stdtypes.html#mapping-types-dict as a basis for this update.

I chose np as a place holder for a NeoPixel instance as it felt appropriate, additionally I updated the code example to use np instead if n so that there was consistency accross the documentation.

I think the Methods sections should also be combined but I have not included that change here but can if you think that is the way to go.

Testing

This is a trivial documentation change so no testing was done

Signed-off-by: tobes <toby.junk@gmail.com>
@@ -54,17 +54,17 @@ Pixel access methods
Sets the value of all pixels to the specified *pixel* value (i.e. an
RGB/RGBW tuple).

.. method:: NeoPixel.__len__()
.. method:: len(np)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no method Neopixel.len

Copy link
Author

Choose a reason for hiding this comment

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

I realise it's not a method but the .. method:: directive is as much about formatting as anything else. What would you suggest. As I say I was using pythons documentation as a guide.

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend to leave the docs unchanged - as changing them will likely break the hyperlinks in the documentation , and surely break the generation of the micropython-stubs, which actually parse the directives.
Please see my comment for recommendations


.. method:: NeoPixel.__setitem__(index, val)
.. method:: np[index]
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no method Neopixel.np

@Josverl
Copy link
Contributor

Josverl commented Feb 16, 2025

The definition and documentation of the dunder methods should not be removed.
If you want to clarify how they can be used, then I recommend to do that as part of the respective method.

@tobes
Copy link
Author

tobes commented Feb 17, 2025

The definition and documentation of the dunder methods should not be removed.

I'm not aware of dunder methods being documented as 'real' methods in any python documentation. They aren't public methods as such because of how you access them. To me it is like saying NeoPixel.__init__() should be documented - it doesn't really make sense.

Part of the python philosophy is to respect private methods and imho the current documentation goes against that.

@Josverl
Copy link
Contributor

Josverl commented Feb 17, 2025

The NeoPixel class has a almost unique combination of runtime properties that AFAIK are not captured as a Protocol nor as a Base Class
Removing these would hide these capabilities.

Als, Python does document dunder methods, just a few examples below.

@tobes
Copy link
Author

tobes commented Feb 17, 2025

Ok, but there seem to be very few dunder methods and mostly around email so it still feels fairly niche. Eg very few __len__() methods are documented considering how many objects support len()

I don't know enough about the NeoPixel code but it still feels odd that the natural way to use an instance isn't documented apart from one line in the code example, which might confuse some users as there is no explanation.

I still feel that these ways to access the object should be documented for users less familiar with python.

@dpgeorge dpgeorge added the docs label Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants