refactor: Rename MatplotlibWriter to MatplotlibDataset
#1027
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
SajidAlamQB
changed the title
MatplotlibWriter to MatplotlibDatasetMatplotlibWriter to MatplotlibDataset
SajidAlamQB
changed the title
MatplotlibWriter to MatplotlibDatasetMatplotlibWriter to MatplotlibDataset
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
…ro-org/kedro-plugins into feat/rename-matplotlibwriter Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
Signed-off-by: Sajid Alam <sajid_alam@mckinsey.com>
| "version": self._version, | ||
| } | ||
|
|
||
| def load(self) -> NoReturn: |
There was a problem hiding this comment.
I think this is not documented well in the older docs as well. It says Loads data by delegation to the provided load method.. In code we raise an error
| save_args: | ||
| format: png | ||
|
|
||
| Example usage for the |
There was a problem hiding this comment.
[Nit] The example usage still uses plot_writer variable names when assigning MatplotlibDataset instances. May be plot_dataset would be better
| "The MatplotlibWriter class has been renamed to MatplotlibDataset. " | ||
| "The MatplotlibWriter name is deprecated and will be removed in a future release.", | ||
| DeprecationWarning, | ||
| stacklevel=2, |
There was a problem hiding this comment.
I tested initializing this class with the example usage provided in the docs. For some reason I am not seeing this deprecation warning. Any snippet to QA the warning ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Related to: #353
This PR implements the of renaming
MatplotlibWritertoMatplotlibDatasetfor consistency with other datasets in the codebase.In the current release:
MatplotlibWriternow inherits fromMatplotlibDatasetand raises deprecation warningsIn a future major release:
MatplotlibWriterwill be completely removed onlyMatplotlibDatasetwill remainDevelopment notes
MatplotlibDatasetclass which is identical toMatplotlibWriterexcept for the name.MatplotlibWriterto inherit fromMatplotlibDatasetwhile adding deprecation warnings.__init__.py.MatplotlibWriter.Checklist
jsonschema/kedro-catalog-X.XX.jsonif necessaryRELEASE.mdfile