feat(charts): add export methods to BaseChart and deprecate Datawrapper.export_chart()#478
feat(charts): add export methods to BaseChart and deprecate Datawrapper.export_chart()#478palewire merged 11 commits intochekos:mainfrom
Conversation
…er.export_chart() Add new export_png(), export_pdf(), and export_svg() methods to the BaseChart class, providing a more intuitive object-oriented API for exporting charts. These methods return raw bytes that can be easily saved to files. Deprecate the legacy Datawrapper.export_chart() method with a warning that directs users to the new OO approach. The deprecated method will be removed in a future version. Changes: - Add export_png/pdf/svg methods to BaseChart with comprehensive parameter support - Include detailed docstrings with examples for each export method - Add DeprecationWarning to Datawrapper.export_chart() with migration guidance - Add comprehensive test coverage for all new export methods - Test error handling for missing chart_id and API failures This change improves the API consistency by allowing users to export charts directly from chart objects rather than through the main Datawrapper class.
Remove the deprecated export() method that was marked for removal in favor of the more flexible export_png(), export_pdf(), and export_svg() methods. These newer methods return raw bytes instead of handling file I/O, providing better flexibility for users to handle the exported data as needed. The export() method had been deprecated since version 2.1.0 and users have been warned to migrate to the new export methods.
There was a problem hiding this comment.
Pull Request Overview
This PR adds dedicated export methods (export_png, export_pdf, export_svg) to the BaseChart class and deprecates the existing Datawrapper.export_chart() method. The new methods return raw bytes instead of file paths, providing a simpler and more flexible API for chart exporting.
Key changes:
- Added three format-specific export methods that return bytes directly
- Deprecated the legacy
export_chart()method on the Datawrapper client - Updated documentation to demonstrate the new export methods
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/test_base_export.py | Comprehensive test suite for the new export methods with parameter validation |
| docs/user-guide/chart-operations.md | Added examples showing how to use the new export methods |
| docs/user-guide/advanced/exporting.md | Removed documentation for the deprecated export_chart method |
| datawrapper/charts/base.py | Implemented export_png, export_pdf, and export_svg methods on BaseChart |
| datawrapper/main.py | Added deprecation warning to export_chart method |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Extract common grid and axis functionality into reusable mixins (GridConfigMixin, GridFormatMixin, CustomRangeMixin, CustomTicksMixin) to reduce code duplication across chart types. This removes 60+ lines of repeated field definitions for custom ranges, custom ticks, and grid formatting properties from AreaChart, LineChart, and MultipleColumnChart. Update serialization logic to use mixin-provided methods for consistent handling of grid configuration, axis formatting, custom ranges, and custom ticks across all chart implementations. This improves maintainability and ensures uniform behavior when serializing chart metadata.
Add comprehensive documentation for the chart mixins module, covering grid configuration (GridConfigMixin, GridFormatMixin) and axis customization (CustomRangeMixin, CustomTicksMixin). Includes usage examples for each mixin demonstrating common chart configuration patterns like controlling grid visibility, formatting labels, and customizing axis ranges and tick marks.
Export CustomRangeMixin, CustomTicksMixin, GridConfigMixin, and GridFormatMixin from the main datawrapper package to make them available for public use. These mixins provide functionality for customizing chart axes, grid display, and formatting. Also updated documentation to use the public import pattern (import datawrapper as dw) instead of importing directly from submodules, making examples more consistent with recommended usage.
Add Pydantic Field descriptors with detailed descriptions to all attributes in chart mixins (GridConfigMixin, GridFormatMixin, CustomRangeMixin, and CustomTicksMixin). This improves API documentation and provides better context for users about what each field controls and how it should be used. Changes: - Import Field from pydantic - Replace simple default assignments with Field() descriptors - Add descriptive documentation for x_grid, y_grid, x_grid_format, y_grid_format, custom_range_x, custom_range_y, custom_ticks_x, and custom_ticks_y attributes - Maintain existing default values and type hints
Rename GridConfigMixin to GridDisplayMixin across the codebase to better reflect its purpose of controlling grid display visibility (x_grid and y_grid fields) rather than general grid configuration. This improves code clarity and naming consistency. Changes: - Renamed class GridConfigMixin to GridDisplayMixin in mixins.py - Updated all imports and references across chart modules (area, column, line, multiple_column) - Updated __all__ exports in __init__.py files - Updated documentation references <response> </response>
Add timeout parameter (default 30s) to export_as_png(), export_as_pdf(), and export_as_svg() methods to allow users to configure request timeouts for chart export operations. This prevents indefinite hangs when the Datawrapper API is slow or unresponsive during export generation.
Remove duplicate code block that was setting width, height, and border_color parameters and making the API request in the PNG export method. The parameters are already being set and the request is being made in the subsequent code block, making this duplication unnecessary.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,69 @@ | |||
| """Base serializer class for all serialization utilities.""" | |||
|
|
|||
| from abc import ABC, abstractmethod | |||
There was a problem hiding this comment.
The BaseSerializer class is created but not exported from the serializers module's __init__.py. Since all serializer classes now inherit from it, the base class should be added to the module's __all__ list for proper API visibility and to allow explicit imports like from datawrapper.charts.serializers import BaseSerializer.
| visualize_data = { | ||
| # Horizontal and vertical axis (from mixins) | ||
| **self._serialize_grid_config(), | ||
| **self._serialize_grid_format(), | ||
| **self._serialize_custom_range(), | ||
| **self._serialize_custom_ticks(), | ||
| # Vertical axis (chart-specific) |
There was a problem hiding this comment.
The comment says 'Horizontal and vertical axis (from mixins)' but the mixin overrides in this class handle ColumnChart-specific field naming. Consider updating the comment to '# Horizontal and vertical axis (from mixins, with ColumnChart-specific overrides)' to clarify that the methods are overridden.
| # Return raw bytes | ||
| if isinstance(response, bytes): | ||
| return response | ||
| raise ValueError(f"Unexpected response type from API: {type(response)}") |
There was a problem hiding this comment.
The error message 'Unexpected response type from API' is not specific enough. It should indicate what type was expected (bytes) to help with debugging. Consider: f\"Expected bytes response from API, got {type(response).__name__}\"
| raise ValueError(f"Unexpected response type from API: {type(response)}") | |
| raise ValueError(f"Expected bytes response from API, got {type(response).__name__}") |
| # Return raw bytes | ||
| if isinstance(response, bytes): | ||
| return response | ||
| raise ValueError(f"Unexpected response type from API: {type(response)}") |
There was a problem hiding this comment.
The error message 'Unexpected response type from API' is not specific enough. It should indicate what type was expected (bytes) to help with debugging. Consider: f\"Expected bytes response from API, got {type(response).__name__}\"
| raise ValueError(f"Unexpected response type from API: {type(response)}") | |
| raise ValueError(f"Expected bytes response from API, got {type(response).__name__}") |
| # Return raw bytes | ||
| if isinstance(response, bytes): | ||
| return response | ||
| raise ValueError(f"Unexpected response type from API: {type(response)}") |
There was a problem hiding this comment.
The error message 'Unexpected response type from API' is not specific enough. It should indicate what type was expected (bytes) to help with debugging. Consider: f\"Expected bytes response from API, got {type(response).__name__}\"
| raise ValueError(f"Unexpected response type from API: {type(response)}") | |
| raise ValueError(f"Expected bytes response from API, got {type(response).__name__}") |
| description="Custom range for X-axis as [min, max]. Overrides automatic range calculation.", | ||
| ) | ||
| custom_range_y: list[Any] | tuple[Any, Any] | None = Field( | ||
| default=None, | ||
| description="Custom range for Y-axis as [min, max]. Overrides automatic range calculation.", |
There was a problem hiding this comment.
The field descriptions say 'as [min, max]' but the type annotation also accepts tuples. Consider updating the description to 'as [min, max] or (min, max)' to match the type signature.
| description="Custom range for X-axis as [min, max]. Overrides automatic range calculation.", | |
| ) | |
| custom_range_y: list[Any] | tuple[Any, Any] | None = Field( | |
| default=None, | |
| description="Custom range for Y-axis as [min, max]. Overrides automatic range calculation.", | |
| description="Custom range for X-axis as [min, max] or (min, max). Overrides automatic range calculation.", | |
| ) | |
| custom_range_y: list[Any] | tuple[Any, Any] | None = Field( | |
| default=None, | |
| description="Custom range for Y-axis as [min, max] or (min, max). Overrides automatic range calculation.", |
| # Make the API request | ||
| response = client.get( | ||
| f"{client._CHARTS_URL}/{self.chart_id}/export/pdf", | ||
| params=params, |
There was a problem hiding this comment.
The export_pdf method is missing the timeout parameter in the client.get() call, while export_png (line 801) and export_svg (line 940) both include it. This inconsistency could lead to unexpected timeout behavior for PDF exports.
| params=params, | |
| params=params, | |
| timeout=timeout, |
No description provided.