-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add a version of pygal.js that can be used as a plain JavaScript ES6 module #12
Open
tuzz
wants to merge
27
commits into
trinketapp:master
Choose a base branch
from
RaspberryPiFoundation:plain-javascript-module
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add a version of pygal.js that can be used as a plain JavaScript ES6 module #12
tuzz
wants to merge
27
commits into
trinketapp:master
from
RaspberryPiFoundation:plain-javascript-module
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
This is Skulpt specific.
Rather than try to anticipate all contexts in which the code is run, write a single ES6 module and let people use Babel if polyfills are required.
If a keyword argument isn’t provided, its value will be undefined.
I don’t think the render method in createChartType is needed for the plain JavaScript implementation. It just seems to be setting values in the _options object but we already do this in the Chart constructor.
This is optimistic. We might need to make some of them mutable again later.
We need to be able instantiate the Chart subclasses with: pygal.Line() When used in Pyodide, you have to call constructors like this: pygal.Line.new() So we need to export functions that instantiate the subclasses, instead.
We need a way to provide the domOutput function and the availableWidth and availableHeight values. Export a config object that allows the caller to set these and print an error if they haven't set domOutput.
I think the previous Skulpt implementation caused option to be objects with a value (v) field whereas now all options are native JavaScript values so we don’t need to do this.
This is pyodide specific but should be a noop for plain JavaScript consumers. In pyodide, some function arguments might be Proxy objects so we first need to convert them to JavaScript objects before use. We don’t need to apply this to everything (e.g. strings, booleans) but I’m worried about missing some argument, e.g. if something is normally a string but Highcharts also allows it to be an array.
tuzz
added a commit
to RaspberryPiFoundation/python-execution-prototypes
that referenced
this pull request
Jan 7, 2024
I think I have finished porting pygal.js from a Skulpt module to a plain JavaScript ES6 module which is now supported in Pyodide. More context: trinketapp/pygal.js#12
In commit 5666fce, I mistakenly thought that the $loc.render method wasn’t doing anything but now I realise it was updating this._options with any top-level attributes that had been set on the chart in Python: ```python bar_chart = pygal.Bar(title="Foo") bar_chart.title = "Bar" ``` In this ^ case, it was fetching the ‘title’ property from the bar_chart instance and setting it in the _options object so that it can be used for rendering. We could repeat this pattern at the top of the Chart#render method but I think it’s more idiomatic to implement getters and setters for all chart properties so that this._options is always up to date, without baving to call Chart#render first. For example, someone might implement another method that depends on title and calls it before #render.
tuzz
added a commit
to RaspberryPiFoundation/python-execution-prototypes
that referenced
this pull request
Jan 8, 2024
I think I have finished porting pygal.js from a Skulpt module to a plain JavaScript ES6 module which is now supported in Pyodide. More context: trinketapp/pygal.js#12
tuzz
added a commit
to RaspberryPiFoundation/python-execution-prototypes
that referenced
this pull request
Jan 9, 2024
Introduce a config object which will be the interface to/from JavaScript. I used the same pattern for my conversion of Pygal: trinketapp/pygal.js#12
We would like to use pygal with pyodide inside a web worker but we are unable to access the DOM in a web worker. Therefore, we need to pass the chart JavaScript object back (using postMessage) and render it in the web page. This interface change makes that possible.
f869b1f
to
3b2e028
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.
Currently the
__init__.js
file is a Skulpt module and uses Skulpt-specific methods to add methods the Python interpreter. We would like to use Pygal with other Python interpreters such as Pyodide. There's no reason Pygal can't be a plain JavaScript ES6 module so this pull request carefully converts it to one, that can then be used with Pyodide or other JavaScript projects.I have tried to be extremely methodical in my commits so that the existing functionality and interface are the same, but it's hard to know for sure because we don't have tests. I have added a new
demo-pyodide.html
example that works correctly so I am somewhat confident the conversion has worked. I have also updated theREADME.md
to explain that you can now use the module directly if you wish.I have also removed the dependency on jQuery (the
$elem
variable was only used in one place). We might want to do this for the Skulpt__init__.js
file but I didn't want to change existing behaviour. Longer term, we might want to see if it's possible to rewrite__init__.js
so that it usespygal.js
to avoid the code duplication but, again, I didn't want to risk breaking existing behaviour.It'd be great if this could be merged into pygal.js, otherwise, we're happy to keep it around as a fork on RaspberryPiFoundation. Thanks