-
Notifications
You must be signed in to change notification settings - Fork 11
Users/zhaodongwang/docstring update #63
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
Conversation
…averseClient-Python into users/zhaodongwang/docstringUpdate
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.
Pull Request Overview
This PR refactors the Dataverse SDK's import structure and updates documentation to improve modularity and clarity. The main changes involve making internal modules private (renaming core to _core and data to _data), enforcing explicit submodule imports, and standardizing docstring type annotations to use Sphinx :class: syntax.
- Renamed internal modules from
coreto_coreanddatato_datato indicate they are private - Removed
DataverseClientfrom top-level__init__.pyto enforce explicit imports fromPowerPlatform.Dataverse.client - Updated all docstrings to use
:class:syntax for type annotations instead of backticks - Created placeholder
__init__.pyfiles for new internal module structure (_utils,_models,_extensions)
Reviewed Changes
Copilot reviewed 18 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/PowerPlatform/Dataverse/init.py | Removed DataverseClient from top-level exports to enforce explicit imports |
| src/PowerPlatform/Dataverse/client.py | Updated docstrings to use :class: syntax and fixed import paths to use _core and _data |
| src/PowerPlatform/Dataverse/_core/*.py | Updated module references from core to _core in docstrings and moved from __future__ import annotations after module docstrings |
| src/PowerPlatform/Dataverse/_data/*.py | Updated import paths to use _core prefix and partially updated docstring type annotations |
| src/PowerPlatform/Dataverse/_core/init.py | Added placeholder init file for private core module |
| src/PowerPlatform/Dataverse/_data/init.py | Added placeholder init file for private data module |
| src/PowerPlatform/Dataverse/_utils/init.py | Added placeholder init file for future utilities module |
| src/PowerPlatform/Dataverse/_models/init.py | Added placeholder init file for future models module |
| src/PowerPlatform/Dataverse/_extensions/init.py | Added placeholder init file for future extensions module |
| tests/unit/data/*.py | Updated test imports to use _data and _core module paths |
| tests/unit/core/test_http_errors.py | Updated test imports to use _core and _data module paths |
| tests/conftest.py | Updated import to use _core.config path |
| examples/basic/*.py | Updated example imports to use explicit DataverseClient import from client module and _core paths |
| examples/advanced/file_upload.py | Updated example import to use explicit DataverseClient import from client module |
| README.md | Updated documentation examples to use new import structure |
Comments suppressed due to low confidence (2)
src/PowerPlatform/Dataverse/_data/odata.py:89
- Inconsistent type annotation: This line still uses the old backtick notation (
``None``) while the rest of the PR updates type annotations to useNonewithout backticks (e.g., see line 36 in client.py::type config: ~PowerPlatform.Dataverse._core.config.DataverseConfig or None). This should be updated to match the new style for consistency.
src/PowerPlatform/Dataverse/_data/odata.py:87 - Inconsistent type annotation: This line still uses the old backtick notation (
``str``) while the rest of the PR updates primitive type annotations to use the:class:syntax (e.g.,:class:str). This should be updated to `:class:`strfor consistency with the other changes in this PR.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…om/microsoft/PowerPlatform-DataverseClient-Python into users/zhaodongwang/docstringUpdate
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@zhaodongwang-msft I've opened a new pull request, #66, to work on those changes. Once the pull request is ready, I'll request review from you. |
Copilot summary:
This pull request refactors the import structure of the Dataverse SDK to improve modularity and clarity, and updates documentation and example code to match the new structure. The main change is that
DataverseClientand core modules are now imported from explicit submodules (such asPowerPlatform.Dataverse.clientandPowerPlatform.Dataverse._core.errors), rather than directly from the top-level package. Additionally, the internal modules have been renamed fromcoreto_coreand documentation strings have been updated for consistency and accuracy.Import and API structure changes:
DataverseClientand core modules in documentation and examples to use explicit submodules, e.g.,from PowerPlatform.Dataverse.client import DataverseClientandfrom PowerPlatform.Dataverse._core.errors import HttpError, MetadataError. (README.md,examples/advanced/file_upload.py,examples/basic/functional_testing.py,examples/basic/installation_example.py) [1] [2] [3] [4] [5] [6] [7] [8]DataverseClientfrom the top-level__init__.pyand restricted__all__to only expose__version__, enforcing use of submodule imports. (src/PowerPlatform/Dataverse/__init__.py)Internal module renaming and docstring updates:
coreto_corefor better encapsulation, and updated all references in docstrings and type annotations to match the new naming convention (e.g.,PowerPlatform.Dataverse._core.auth). (src/PowerPlatform/Dataverse/_core/auth.py,src/PowerPlatform/Dataverse/_core/config.py,src/PowerPlatform/Dataverse/_core/errors.py,src/PowerPlatform/Dataverse/_core/error_codes.py,src/PowerPlatform/Dataverse/_core/http.py) [1] [2] [3] [4] [5]Type annotation and docstring improvements:
:class:syntax for clarity and Sphinx compatibility, replacing old backtick notation. (src/PowerPlatform/Dataverse/_core/auth.py,src/PowerPlatform/Dataverse/_core/config.py,src/PowerPlatform/Dataverse/_core/errors.py,src/PowerPlatform/Dataverse/_core/error_codes.py) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]These changes make the SDK's structure more maintainable and its usage clearer for developers.