Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

fix: Try updating Pyodide #1642

Merged
merged 8 commits into from
Apr 27, 2022
Merged

fix: Try updating Pyodide #1642

merged 8 commits into from
Apr 27, 2022

Conversation

faucomte97
Copy link
Contributor

@faucomte97 faucomte97 commented Apr 26, 2022

This change is Reviewable

@faucomte97 faucomte97 self-assigned this Apr 26, 2022
@faucomte97 faucomte97 marked this pull request as ready for review April 27, 2022 15:43
@faucomte97 faucomte97 changed the title fix: Try updating Pyodide fix: Try updating Pyodide Apr 27, 2022
@faucomte97 faucomte97 changed the title fix: Try updating Pyodide Try updating Pyodide Apr 27, 2022
@faucomte97 faucomte97 changed the title Try updating Pyodide fix: Try updating Pyodide Apr 27, 2022
Copy link
Collaborator

@razvan-pro razvan-pro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @faucomte97)


game_frontend/src/global.d.ts line 2 at r1 (raw file):

declare var OnetrustActiveGroups: string
declare function loadPyodide(any: any): Promise<any>

We could keep the Pyodide interface and make it return Promise<Pyodide>? Also, the argument could be called options: any 🙂


game_frontend/src/pyodide/webWorker.ts line 14 at r1 (raw file):

  importScripts('https://cdn.jsdelivr.net/pyodide/v0.20.0/full/pyodide.js')
  pyodide = await loadPyodide({
    indexURL: "https://cdn.jsdelivr.net/pyodide/v0.20.0/full/"

Oh, I think we can remove this indexUrl and just call it without any options 🙂


game_frontend/src/pyodide/webWorker.ts line 32 at r1 (raw file):

from js import Object
from pyodide import to_js
import sys

Organise imports 🙂

Copy link
Contributor Author

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @razvan-pro)


game_frontend/src/global.d.ts line 2 at r1 (raw file):

Previously, razvan-pro (Razvan Mahu) wrote…

We could keep the Pyodide interface and make it return Promise<Pyodide>? Also, the argument could be called options: any 🙂

Done.

Code quote:

interface Pyodide {
  loadPackage(packages: string[]): Promise<void>
  runPythonAsync(pythonCode: string): Promise<any>
}

game_frontend/src/pyodide/webWorker.ts line 14 at r1 (raw file):

Previously, razvan-pro (Razvan Mahu) wrote…

Oh, I think we can remove this indexUrl and just call it without any options 🙂

Done.


game_frontend/src/pyodide/webWorker.ts line 32 at r1 (raw file):

Previously, razvan-pro (Razvan Mahu) wrote…

Organise imports 🙂

Done.

Copy link
Collaborator

@razvan-pro razvan-pro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @faucomte97)

@codecov
Copy link

codecov bot commented Apr 27, 2022

Codecov Report

Merging #1642 (f544d64) into master (9a2cc11) will increase coverage by 0.02%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1642      +/-   ##
==========================================
+ Coverage   67.96%   67.98%   +0.02%     
==========================================
  Files         164      164              
  Lines        3328     3327       -1     
  Branches      280      280              
==========================================
  Hits         2262     2262              
+ Misses       1038     1037       -1     
  Partials       28       28              
Impacted Files Coverage Δ
game_frontend/src/pyodide/webWorker.ts 22.22% <0.00%> (+0.60%) ⬆️
...ntend/src/redux/features/AvatarWorker/operators.js 100.00% <100.00%> (ø)

@faucomte97 faucomte97 merged commit 4931148 into master Apr 27, 2022
@faucomte97 faucomte97 deleted the fix_pyodide branch April 27, 2022 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants