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

Minor correction to the Read The Docs #24

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

Timeline8
Copy link
Contributor

@Timeline8 Timeline8 commented Jul 19, 2024

Under the I2C class for EEPROM parameters currently reads as "max_int – (Optional) " should be "max_size (int) - (Optional)"

Also I didn't know how to do it, but in the read the docs page the " Default is _MAX_SIZE_I2C " part reads out at literally "_MAX_SIZE_I2C" instead of like it does above in the class

class adafruit_24lc32.EEPROM_I2C(i2c_bus: busio.I2C, address: int = 80, write_protect: bool = False, wp_pin: DigitalInOut | None = None, max_size: int = 4096)¶

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thanks! I'm not sure what you are asking about.

@tannewt tannewt merged commit 1723971 into adafruit:main Jul 22, 2024
1 check passed
@FoamyGuy
Copy link
Collaborator

I think maybe @Timeline8 was referring to the way the docs render the max_size argument default:
image

In the function signature it seems to be able to figure out the value of _MAX_SIZE_I2C is 4096 and show that. Whereas below in the vertical list of arguments it shows the variables name "_MAX_SIZE_I2C" instead of it's value.

I don't know why RTD / sphinx is able to fill in the variable value for one and not the other. We could hardcode 4096 in the docstring if we wanted then the docs would show it the same in both places, but if the value of the variable were ever changed in this library or one copied from it that value could get stale. Maybe that is not that big of a risk though.

@Timeline8
Copy link
Contributor Author

Timeline8 commented Jul 23, 2024

Looks like the main point I posted got fixed (was it my edit?). where it now correctly shows in the Read the docs that it should be max_size (int) ...

maxsize

it was saying "max_int – (Optional)" which was nonsense.

@Timeline8
Copy link
Contributor Author

Here was my edit...

MaxSizeedit

@Timeline8
Copy link
Contributor Author

Timeline8 commented Jul 23, 2024

And my secondary comment was how these two didn't seem to mesh correctly (as @FoamyGuy correctly pointed out)...

Maxsizeread

Where in the "class" line at the top is properly shows the default as 4096 but in the text below it gives the vague "_MAX_SIZE_I2C" rather than an integer number.

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jul 23, 2024
@tannewt
Copy link
Member

tannewt commented Jul 24, 2024

Ah! Ya, signature information is determined by actually loading the code. The doc string isn't evaluated at all. That's also why address is 80 in the signature but 0x50 in the docs. You could remove the mention of the default in the doc string completely because that info is in the signature already.

@Timeline8 Timeline8 deleted the doc_fix_line_226 branch August 23, 2024 02:08
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.

3 participants