-
Notifications
You must be signed in to change notification settings - Fork 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
v0.4.0 API Simplifications and Uniformity Changes #27
Merged
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
jentfoo
changed the title
v0.4.0 Config API Changes
v0.4.0 Config API Simplifications and Uniformity Changes
Jan 1, 2025
jentfoo
commented
Jan 1, 2025
jentfoo
commented
Jan 1, 2025
jentfoo
commented
Jan 1, 2025
jentfoo
commented
Jan 1, 2025
jentfoo
commented
Jan 1, 2025
jentfoo
commented
Jan 1, 2025
jentfoo
commented
Jan 1, 2025
jentfoo
commented
Jan 1, 2025
jentfoo
force-pushed
the
jent/v0.4.0-config_api
branch
5 times, most recently
from
January 3, 2025 19:58
8c80514
to
477c66a
Compare
jentfoo
commented
Jan 4, 2025
jentfoo
commented
Jan 4, 2025
jentfoo
commented
Jan 4, 2025
jentfoo
force-pushed
the
jent/v0.4.0-config_api
branch
from
January 10, 2025 04:27
8c4b45c
to
34619ec
Compare
jentfoo
commented
Jan 10, 2025
jentfoo
force-pushed
the
jent/v0.4.0-config_api
branch
3 times, most recently
from
January 10, 2025 06:13
b380f4f
to
0b28abc
Compare
jentfoo
changed the title
v0.4.0 Config API Simplifications and Uniformity Changes
v0.4.0 API Simplifications and Uniformity Changes
Jan 10, 2025
jentfoo
force-pushed
the
jent/v0.4.0-config_api
branch
from
January 11, 2025 20:40
95193fb
to
b0a39e6
Compare
v0.4.0 will be focused on minimizing and simplifying the public API so that our module is easier to learn. This will include removing public structs and functions not expected to be utilized by the user, as well as making sure the public API presented is consistent. Changes included so far: * `LegendOptions.Vertical` introduced in #20 has been changed from a `bool` to `*bool`. This makes the field more consistent to other configuration, and provides more future flexibility for dynamic defaults. * Public `NewXPainter` functions are made private, these are only expected to be used internally. * Public structs `AxisOption`, `GridPainterOption`, `SeriesLabelPainter`, `SeriesLabelPainterParams`, have been made private or removed. These are expected to only be used internally. * `LabelFormatter` has been removed (not to be confused with `ValueFormatter`, which remains) * Additional functions expected to be internal only: `NewRange`, `NewLeftYAxis`, `NewRightYAxis`, `NewBottomXAxis`, `NewEChartsSeriesDataValue` (struct is still public if needed) * `PainterOption` function has been renamed to `PainterOptionFunc` * Removed `OutputFormatOptionFunc`, instead use `SVGOutputOptionFunc()` or `PNGOutputOptionFunc()` * Dimensions are now always set together, `WidthOptionFunc` and `HeightOptionFunc` are now set with `DimensionsOptionFunc`. Similar `SetDefaultWidth` and `SetDefaultHeight` were replaced with `SetDefaultChartDimensions`. * `BoxOptionFunc` was removed, we don't currently have a clear example of where this is useful.
Functions on `Painter` have been reduced to only post-rendered chart modifications a user may want to do. For example, drawing arrows, adding text, additional lines, circles or boxes, etc. As part of this `ArrowTop` and `ArrowBottom` were renamed to `ArrowUp` and `ArrowDown`.
This was previously identified as internal, but on closer inspection I think it should be removed. This is not used by the table creation, so I would like a cleaer understanding of how it's used (and add an example of it) before continuing to support it.
This struct only contained a single field `Value` which was a float64. To reduce the API surface area this struct is being removed and instead the `Series` struct will contain a slice of `float64` directly for the `Data`.
The prior TODO note to make the fields lower case is not happening in this version. Instead we want to help elevate these statistics in case they are useful to the user.
This removes from the public API NewSeriesListDataFromValues and NewSeriesFromValues. Instead of optionally passing in a chart type it's now expected to call a function specific to the chart type: NewSeriesListLine, NewSeriesListBar, NewSeriesListHorizontalBar, NewSeriesListPie, NewSeriesListRadar, or NewSeriesListFunnel. NewPieSeriesList and NewFunnelSeriesList previously existed, so this API makes the construction more uniform for all chart types (note that these have been renamed to have a `NewSeriesList` prefix to the function name). While examining the Options struct passed in (which has now been adopted for all series types), it was noted that Pie rendering almost always passed in a SeriesLabel to enable rendering the labels. The examples demonstrate these API changes well, hopefully this API is more concise and easier to learn.
These fields were only used in the Funnel charts, and the usage not currently tested. Removing this functionality until it's use case is clearly understood and tested.
This removes the error return on `chartdraw.RendererProvider`. Currently the only error return was for a PNG case that was due to a type checking that would have been from a bug. Instead the function signatures were changed so the type check was no longer necessary, making the error result no longer needed. The above change was able to cascade until `charts.NewPainter` no longer needed to return an error result either.
I don't like that this API implied an immutability in the Painter that did not exist. Code can be easily restructured to call into the Painter reference sequentually (Minus the `Child` action, which does in fact return a new instance)
This commit removes the need for external users to be aware of Renders (`Renderer` type made private in a prior commit). Instead if a user wants to render charts on a Painter they can invoke the relevant function on the painter: * NewBarChart -> Painter.BarChart * NewHorizontalBarChart -> Painter.HorizontalBarChart * NewFunnelChart -> Painter.FunnelChart * NewLineChart -> Painter.LineChart * NewPieChart -> Painter.PieChart * NewRadarChart -> Painter.RadarChart * NewTableChart -> Painter.TableChart In additional `TableOptionRender` was renamed to `TableOptionRenderDirect` and `TableRender` was renamed to `TableRenderValues`, both to provide clarity because these render functions are unique compared to others.
This name seems more obvious to match with the more generalized `ChartOptions`
The axisOption struct is internal, only used to minimize code duplication between the x and y axis.
jentfoo
force-pushed
the
jent/v0.4.0-config_api
branch
2 times, most recently
from
January 12, 2025 04:50
1a53809
to
cd51aaf
Compare
Changes put together because both required significant changes to the SVG validation in tests. These changes were carefully examined to make sure there were no negative visual issues. **Painter API changes**: The Painter API can be confusing, with some functions depending on the state of the painter. This changes the Painter API (internal and external) so that style attributes (ie FillColor, StrokeColor, StrokeWidth, etc) are not stateful. Instead the relevant style values are accepted as arguments to the draw action. This creates an easier to use API. It also addresses a complexity if multiple Child painters are used together with different drawing state. Additionally the Painter configuration has changed. A default theme and default font is still available to set in the Painter, however the `ValueFormatter` was removed. Instead this config field only exists in charts options which make use of a set formatter. Test SVG's resulted from two primary aspects of this change: * Measuring text not representing the actual text size due to the font style not set correctly in the Painter * A fill style attribute being specified when it has no effect (since style was set due to prior operations) In general the exposed functions were general examined to ensure all exposed functions are both useful, and easy to use. This resulted in a number of argument changes, which should be detailed in the go docs. **vector_renderer (SVG) improvements**: This change minimizes and simplifies the SVG output through the following: * If stroke width is zero, or the color is transparent only `stroke:none` will be set, instead of the prior `stroke-width:0;stroke:none` value * Colors will be specified by name if possible, otherwise an RGB or RGBA string will still be used * If the text or path is empty, nothing will be inserted into the svg. These three changes reduce the SVG size. Specifying the color names could potentially allow color shade changes using CSS. As a bonus, additional colors are now specified in the `drawing` package. ---- SVG changes are always painful, these were careful evaluated to ensure this is an improvement in both the Painter functionality and SVG results.
This change solves bringing the default functionality more equal with both API's. * Introduced Option struct constructors for each chart type - These constructor functions makes it easier to setup defaults, as well as call the specific function needed to build the SeriesList for the struct type. * Moved chart series and legend setup to `defaultRender` - defaultRender is a common logic point we should use, with fillDefaults being very similar to the constructors introduced above. * defaultRenderOption now has pointers for structs modified in rendering (another place for defaults filled in). This internal API could likely be improved long term, but this is a small change to help ensure logic remains consistent. Overall this change resolves #30
jentfoo
force-pushed
the
jent/v0.4.0-config_api
branch
from
January 12, 2025 05:46
cd51aaf
to
26851a3
Compare
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.
v0.4.0 will be focused on minimizing and simplifying the public API so that our module is easier to learn. This will include removing public structs and functions not expected to be utilized by the user, as well as making sure the public API presented is consistent.
Although these changes look significant, what is expected to be user impacting API changes should be demonstrated by the changes in
examples
. We expect the user impacts to be very small, with most of the API removal to be functionality not expected to be used externally.If anything removed or hidden in this PR / release is useful to you, please open an issue referencing this PR. The API cleanup is expected only to make it easier to learn and adopt our library, not reduce functionality.
Config Changes:
LegendOptions.Vertical
introduced in Offset, Orientation and other config ergonomict tweaks #20 has been changed from abool
to*bool
. This makes the field more consistent to other configuration, and provides more future flexibility for dynamic defaults.LineChartOption.StrokeWidth
has been renamed toLineStrokeWidth
to be more consistent with other configurations.OutputFormatOptionFunc
, instead useSVGOutputOptionFunc()
orPNGOutputOptionFunc()
WidthOptionFunc
andHeightOptionFunc
are now set withDimensionsOptionFunc
. SimilarSetDefaultWidth
andSetDefaultHeight
were replaced withSetDefaultChartDimensions
.Series Changes:
SeriesList
are now constructed in a more consistent way with a function specific to the type. This removes from the public APINewSeriesListDataFromValues
andNewSeriesFromValues
. Instead of optionally passing in a chart type it's now expected to call a function specific to the chart type:NewSeriesListX
(with X being the chart type). Though manual SeriesList construction is less needed, in general chart Options structs are easier to construct by usingNewXChartOptionWithData
(for example:TestNewLineChartOptionWithData
).SeriesData
struct has been removed, instead we just use afloat64
slice inSeries
to reference the values directly.Painter Changes:
NewXPainter
functions are made private, except forNewPainter
, these other cases are expected to be used internally.NewPainter
there is now no error returnRenderer
type has been made private, andNew
functions to create these structs has been removed. For rendering charts using the Painter API, these functions are now invoked on aPainter
instance, for example:NewBarChart
->Painter.BarChart
NewHorizontalBarChart
->Painter.HorizontalBarChart
NewFunnelChart
->Painter.FunnelChart
NewLineChart
->Painter.LineChart
NewPieChart
->Painter.PieChart
NewRadarChart
->Painter.RadarChart
NewTableChart
->Painter.TableChart
Painter
have been reduced to only post-rendered chart modifications a user may want to do. For example, drawing arrows, adding text, additional lines, circles or boxes, etc. As part of thisArrowTop
andArrowBottom
were renamed toArrowUp
andArrowDown
. Additionally functions on thePainter
don't return aPainter
instance unless the action only applies to the returned instance.Painter
(ie fill color, stroke color, stroke width, etc) is no longer stateful, instead these style attributes are accepted as function arguments. In general functions have changed on the Painter to make it more obvious what is useful for external users.Other Removals and Changes:
LabelFormatter
has been removed (not to be confused withValueFormatter
, which remains)BoxOptionFunc
was removed, we don't currently have a clear example of where this is useful.AxisOption
(and similar theToAxisOption
function),GridPainterOption
,SeriesLabelPainter
,SeriesLabelPainterParams
, have been made private or removed. These are expected to only be used internally.Grid
painter functionality was removed (not needed internally, and not clear how it would be useful to the end user)