Skip to content

DOC: Adding docstrings to functions #15580 #52774

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

Closed
wants to merge 10 commits into from
Closed

DOC: Adding docstrings to functions #15580 #52774

wants to merge 10 commits into from

Conversation

wcgonzal
Copy link
Contributor

Addresses issue #15580 in the Pandas library
Adding docstrings with examples to
using_copy_on_write() and
using_nullable_dtypes() functions, pathway

Files under C:\Users\XXXX\pandas\pandas_config|__init__py

@wcgonzal
Copy link
Contributor Author

Please reach out with any questions or edits you think I should make.

@wcgonzal wcgonzal closed this Apr 19, 2023
@wcgonzal wcgonzal reopened this Apr 19, 2023
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

thanks @wcgonzal

could you fixup the pre-commit failures please? check the contributing guide for how

also, please check the pandas docstring guide (also in the contributing guide)

@wcgonzal
Copy link
Contributor Author

I have made the requested changes and ready for the review process to continue.

Accidentally added an unnecessary file to my pull request.
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

thanks for working on this! almost there 💪

Comment on lines +42 to +45
Parameters
----------
None

Copy link
Member

Choose a reason for hiding this comment

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

is this used elsewhere in the docs? if not, I think we can just leave the parameters section out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

Comment on lines +54 to +60
>>> pd.set_option('mode.chained_assignment', 'raise')
>>> pd.set_option('mode.copy', True)
>>> pd.set_option('mode.use_inf_as_na', True)
>>> pd.set_option('compute.use_bottleneck', False)
>>> pd.set_option('compute.use_numexpr', False)
>>> using_copy_on_write()
True
Copy link
Member

Choose a reason for hiding this comment

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

there's a couple of issues here:

  1. most of these options aren't relevant to this example
  2. the options will stay set after the doctest for this function has finished (there have been issues due to this in the past)

How about we just put something like

>>> pd.set_option('mode.copy', True)
>>> using_copy_on_write()
True
>>> pd.unset_option('mode.copy')
>>> using_copy_on_write()
False

(I haven't tried running this, so the syntax might not be correct)


Example
-------
>>> pd.set_option('mode.use_inf_as_na', True)
Copy link
Member

Choose a reason for hiding this comment

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

can we remove this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

This function assumes that the global pandas configuration settings have
already been initialized. If you are not sure whether the settings
have been initialized, you can call the
pandas.api.types.is_extension_type() function to force initialization.
Copy link
Member

Choose a reason for hiding this comment

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

does is_extension_type initialise the configuration settings? I don't remember having come across this, could you please let me know where you found it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake, it doesn't initialize or make changes for that matter on the Settings.
After some reasearch, pandas.api.extensions.initialize() is more appropriate in this case. Do you recommend I edit this not or is the note really necessary?

Copy link
Member

Choose a reason for hiding this comment

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

what's pandas.api.extensions.initialize? where did you get this from?

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

If you are not sure whether the settings
have been initialized, you can call the
pandas.api.types.is_extension_type() function to force initialization.

My mistake, it doesn't initialize or make changes for that matter on the Settings.
After some reasearch, pandas.api.extensions.initialize() is more appropriate in this case.

Re-reading these nonsense (with all due respect) comments, I'm fairly certain you're just copy-and-pasting from a chatbot

These functions aren't user-facing anyway

Closing for now then, feel free to take on other good first issues (so long as you write the text yourself)

@wcgonzal
Copy link
Contributor Author

wcgonzal commented May 2, 2023

No offense taken, I am using chatbot as a guide but not the only resource I am using to understand the functions.
I'll resubmit later once I fully know the ins and outs of the functions if you don't mind.

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