-
Notifications
You must be signed in to change notification settings - Fork 0
DOC: set __module__ on remaining top-level functions #2
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
base: trunk2
Are you sure you want to change the base?
Conversation
Co-authored-by: Richard Shadrach <rhshadrach@gmail.com>
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. You can monitor the review status in the checks section at the bottom of this pull request. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
|
|
/q review |
|
Thank you for the opportunity to review this pull request. I've examined the changes, and here are my observations:
Overall, the changes look good and achieve the stated goal. Do you have any specific areas you'd like me to focus on or any concerns you'd like addressed? |
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. You can monitor the review status in the checks section at the bottom of this pull request. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
|
|
/q review |
|
Thank you for your work on setting the
Overall, this change appears to be a positive step towards a more organized and consistent pandas API. Great job on improving the project's structure! Do you have any questions or need clarification on any part of this review? |
|
/q review |
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. You can monitor the review status in the checks section at the bottom of this pull request. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this pull request makes good progress in improving the module attribution for functions in the pandas/_libs/lib.pyx file. The consistent application of the @set_module("pandas.api.types") decorator to functions like is_scalar, is_iterator, is_float, and others is a positive change. It ensures these functions are correctly associated with the pandas.api.types module, which is important for maintaining a clear and organized public API.
Suggestions for improvement:
- Review the entire file to ensure all functions that should be part of the public API are decorated with
@set_module. - For functions intentionally left without the decorator, consider adding comments explaining why they are not part of the public API.
- Ensure consistency in applying this pattern across other relevant files in the pandas library.
These changes contribute to better organization and clarity in the pandas codebase. Good job on improving the module attribution!
Coming soon: You'll be able to request the agent to implement changes and create commits on your pull request's source branch. This may trigger any configured CI/CD workflows automatically.
You can disable any CI/CD workflow that you have configured to run on the source branch of the pull request. For more information, see Disabling and enabling a workflow in the GitHub documentation.
| # import set_module here would cause circular import | ||
| get_option.__module__ = "pandas" | ||
| set_option.__module__ = "pandas" | ||
| describe_option.__module__ = "pandas" | ||
| reset_option.__module__ = "pandas" | ||
| option_context.__module__ = "pandas" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes made here are setting the __module__ attribute for several functions to "pandas". This is a good practice for ensuring that these functions are correctly associated with the pandas package. However, we should consider using the @set_module decorator for consistency and maintainability.
| # import set_module here would cause circular import | |
| get_option.__module__ = "pandas" | |
| set_option.__module__ = "pandas" | |
| describe_option.__module__ = "pandas" | |
| reset_option.__module__ = "pandas" | |
| option_context.__module__ = "pandas" | |
| from pandas.util._decorators import set_module | |
| @set_module("pandas") | |
| def get_option(pat: str) -> Any: | |
| ... | |
| @set_module("pandas") | |
| def set_option(*args) -> None: | |
| ... | |
| @set_module("pandas") | |
| def describe_option(pat: str = "", _print_desc: bool = True) -> str | None: | |
| ... | |
| @set_module("pandas") | |
| def reset_option(pat: str) -> None: | |
| ... | |
| @set_module("pandas") | |
| def option_context(*args) -> Generator[None]: | |
| ... |
| from pandas._config import using_string_dtype | ||
|
|
||
| from pandas._libs.missing import check_na_tuples_nonequal | ||
| from pandas.util._decorators import set_module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good addition of the set_module import. This will help in setting the correct module for the decorated functions. However, we should consider applying this decorator consistently to all relevant functions in this file.
| @set_module("pandas.api.types") | ||
| def is_scalar(val: object) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @set_module("pandas.api.types") decorator is correctly applied to the is_scalar function. This ensures that the function is properly associated with the pandas.api.types module. Consider applying this decorator consistently to all relevant functions in this file that should be part of the public API.
| @set_module("pandas.api.types") | ||
| def is_iterator(obj: object) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @set_module("pandas.api.types") decorator is correctly applied to the is_iterator function. This is consistent with the previous application to is_scalar. Ensure that all functions intended to be part of the public API are decorated similarly.
| @set_module("pandas.api.types") | ||
| def is_float(obj: object) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @set_module("pandas.api.types") decorator is consistently applied to the is_float function here, and to other functions like is_integer, is_bool, is_complex, is_list_like, and infer_dtype in the subsequent changes. This is good practice and ensures these functions are correctly associated with the pandas.api.types module.
Consider reviewing the entire file to ensure all relevant functions that should be part of the public API are decorated appropriately. If there are functions intentionally left without the decorator, it might be helpful to add a comment explaining why they are not part of the public API.
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. You can monitor the review status in the checks section at the bottom of this pull request. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
|
Follow-up on pandas-dev#62727, resolving this for the remaining functions in the top-level namespace (and for some of the submodules, where it were only a few ones to address, i.e. pandas.testing and pandas.api.extensions)
xref pandas-dev#55178