go-plotly icon indicating copy to clipboard operation
go-plotly copied to clipboard

update to plotly version 2.29.0

Open PatrickVienne opened this issue 1 year ago • 10 comments

PatrickVienne avatar Feb 09 '24 02:02 PatrickVienne

@MetalBlueberry thanks for the comments, back to you.

PatrickVienne avatar Apr 13 '24 20:04 PatrickVienne

Okey, that last commit is an attempt to reduce the number of changes on this PR.

I don't think we have to update go.mod version as the code doesn't require any new feature. It is good to leave it as is and only update dependencies if we identify a vulnerability or we need some functionality.

The idea is that this will allow more people to use the library without worring about updating things.

Also, In the readme you add instructions to bump to a specific version, Do you think we could write that in a way that just instructs people how to update to ANY version? The process should be the same.

Last but not least, I tried to generate the code from this branch and I noticed the area_gen.go file is not generated. I couldn't investigate why it happen nor if it is expected, it just looks odd as I would expect all the previous types to remain valid. I will look into this once I find time for the project.

Thank you for the contribution!!

MetalBlueberry avatar Apr 14 '24 15:04 MetalBlueberry

Hi @MetalBlueberry

it is not generated, because it is not part of the json schema:

  • go.mod -> Agreed

  • add instructioons: ->

These are the new trace types: jq '.schema.traces | keys' schema.json --sort-keys

[
  "bar",
  "barpolar",
  "box",
  "candlestick",
  "carpet",
  "choropleth",
  "choroplethmapbox",
  "cone",
  "contour",
  "contourcarpet",
  "densitymapbox",
  "funnel",
  "funnelarea",
  "heatmap",
  "heatmapgl",
  "histogram",
  "histogram2d",
  "histogram2dcontour",
  "icicle",
  "image",
  "indicator",
  "isosurface",
  "mesh3d",
  "ohlc",
  "parcats",
  "parcoords",
  "pie",
  "pointcloud",
  "sankey",
  "scatter",
  "scatter3d",
  "scattercarpet",
  "scattergeo",
  "scattergl",
  "scattermapbox",
  "scatterpolar",
  "scatterpolargl",
  "scattersmith",
  "scatterternary",
  "splom",
  "streamtube",
  "sunburst",
  "surface",
  "table",
  "treemap",
  "violin",
  "volume",
  "waterfall"
]

Or via jsonviewer: https://jsonviewer.stack.hu/ image

If this update goes through, I'll submit the next update to the latest version, watching out for your current comments. I also plan to add dark theme, as in plotly for python

I added a downloader script a description how to generate the new files without manual work, only using the scripts. Also added some more hints in the readme on using the scripts and tools like jq and jsonviewer.

Hope this helps to get it through the finish line :) Thanks!

PatrickVienne avatar Apr 20 '24 14:04 PatrickVienne

Alright! I added a few steps to the CI to automatically check that the scheme is correctly built and I've identified some bugs with consistency on the scheme generation :sweat_smile:

Not perfect yet, But I've merged those changes here so we can get better feedback on your changes.

MetalBlueberry avatar May 01 '24 15:05 MetalBlueberry

Thanks a lot @MetalBlueberry anything I can do in support?

PatrickVienne avatar May 01 '24 16:05 PatrickVienne

This PR is making me think about how can we support multipe plotly versions? It is true that generating a new schema is straight forward, but it makes it hard for people just wanting to import the package an use it.

One idea that crossed my mind, given that you already wrote a script to download the schema from plotly site, we cloud write a script that downloads multiple schemas to a directory in this repository and then pair it with the generate script so we can generate all the schemas at once.

The proposal is to have a file strucuture that looks like

  • generated/2.29/graph_objects
  • generated/2.18/graph_objects
  • and so on...

This way, any user can import the latest version of this library but still use any plotly version of their choice.

I don't think we have to support all versions, for a first approach I'll sugest to just support 2.29 and 2.18. So the task you can do is.

Task

Write a script that reads a file in the root of the repository with a list of versions that download schemas to a directory called schemas/2.29.0.json and then have another script that generates the respective go schema on a directory generated/2.29/graph_objects

The file will be schemas.yaml and will contain a key versions with a list of object with the necessary information. For example

versions:
  -  Name: Plotly 2.29.0
     URL: https://whatever
     Path: schemas/2.29.0.json
     Generated: generated/2.29/graph_objects

Design decissions

  • The path ends with graph_objects so all versions can be imported with the same name. This makes it very easy to switch between compatible versions.
  • The Schema Path and the Generate Path are part of the struct so we can update schemas but keep the same output directory. useful to just update patch versions.
  • The file with the list of versions allows to decide which versions to support and increase the values as we need it.
  • Keeping the schema on this repository is needed so we don't have to rely on any external service during CI. The download step must be executed by an human and added to the repository with a PR
  • Generation step must be executed on every commit on the CI

Keep in mind this is a suggestion, feel free to sugest any improvements or make changes. But my advice is to keep it as simple as possible.

This will remove the need to add instructions to the readme about how to run the generator and simply it to Just add the schema to the right directory

MetalBlueberry avatar May 01 '24 20:05 MetalBlueberry

@MetalBlueberry

great idea. Just slightly adjusted the yaml.

Hope this captures the essence of what you meant

running the downloader will download all the schema files as defined in the yaml.

running the generator still needs to be done for each version one by one, but can be done based on the downloaded files in the folder.

image

slightly changed the schema yaml to make the versions more in line with the git tags of the plotly repo image

won't have time for the next weeks to finish it unfortunately, but maybe this can get it started.

open todos still:

  • adjust the readme file
  • also create the plot.go file for separate versions if needed
  • integrate CI generation of newly added versions
  • read the yaml also in the generator, and use the output path of schemas.json to determine the output path.

PatrickVienne avatar May 02 '24 00:05 PatrickVienne

@MetalBlueberry completed all the points, and the generation now is also running in the workflow which you setup for testing

Also updated the readme now and added some release notes.

### Add version v2.29.1 and v2.31.1
- Added Downloader for schema file
- added generator for different packages based on plotly version in subfolders for each version
- enable easier use for different plotly version by new import paths which include the version generated. Example: `grob "github.com/MetalBlueberry/go-plotly/generated/v2.31.1/graph_objects"`
- removed go generate directive from the templates which are no longer necessary
- removed offline package, as each version package needs its own **plot.go**.
- Replaced all `offline` imports in the examples packages by just calling the `grob` package of the specified version
- all html's will now refer to the correct plotly version's cdn in the html's head

We've come a long way, thanks for your comments, really cool stuff, hope we can get this merged. and have next steps implemented in nxt PRs soon. Animations Support and Dark Theme are on my list.

image

PatrickVienne avatar May 11 '24 18:05 PatrickVienne

@MetalBlueberry I can't get the wasm example to run though.

one would also need to make sure to use the correct wasm_exec.js https://github.com/golang/go/blob/go1.21.0/misc/wasm/wasm_exec.js

but this again is a topic for another PR.

PatrickVienne avatar May 11 '24 20:05 PatrickVienne

fixed another missing feature now, which prevented users from using the "arrayOk" feature.

image

		attr := attr[name]
		// if arrays are allowed, example marker sizes, then javascript allows multiple different inputs hard to map in golang:
		// [100,50,25,25,50,100] -> use different sizes for the markers
		// [100] -> only the first marker will have size 100, all others have 0 size
		// 100 -> all markers have size 100
		// this can only be captures by using interface and allowing the user to define the used type themselves.
		// this sadly means that there is no more type checking on those entries
		var ajust = func(typename string) string { return typename }
		if attr.ArrayOK {
			ajust = func(typename string) string { return "interface{}" }
		}

PatrickVienne avatar May 12 '24 13:05 PatrickVienne

@MetalBlueberry back to you

PatrickVienne avatar May 18 '24 22:05 PatrickVienne

@MetalBlueberry frames are removed. back to you

PatrickVienne avatar May 24 '24 22:05 PatrickVienne

Alright, I made a few tweaks.

  • I removed the changes to the offline package, you can submitt that on a different PR
  • Removed some information from the read me, as I feel that information doesn't belong there. It should be obvious from looking at the code structure. If you still feel that information is useful, it can be added on a different location as specific documentation.
  • Fixed the offline package to have the right import paths and CDN
  • Fixed other small things

If you are happy, I'm happy with this. We could merge it

MetalBlueberry avatar May 25 '24 12:05 MetalBlueberry

Thanks. One small cleanup comment, and then let's merge it.

PatrickVienne avatar May 25 '24 12:05 PatrickVienne

Let's Go!!

MetalBlueberry avatar May 25 '24 15:05 MetalBlueberry