Skip to content
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 package Cartopy #3909

Merged
merged 14 commits into from Jun 14, 2023
Merged

Add package Cartopy #3909

merged 14 commits into from Jun 14, 2023

Conversation

juntyr
Copy link
Contributor

@juntyr juntyr commented Jun 6, 2023

Description

Cartopy is a commonly used library to draw maps with matplotlib. Since most of its dependencies are already in pyodide, except for the pure Python dependency pyshp, adding cartopy turned out to be quite simple.

Cartopy often downloads shape files the first time that the user requires them during plotting, which is only supported if the pyodide-http patches are applied. How should this be handled?

Checklists

  • Add a CHANGELOG entry
  • Add / update tests
  • Add new / update outdated documentation

@juntyr
Copy link
Contributor Author

juntyr commented Jun 7, 2023

Mmmm, I'm not sure if I understand why the tests fail - could someone perhaps give me a pointer in the right direction? Thanks :)

@juntyr
Copy link
Contributor Author

juntyr commented Jun 8, 2023

@hoodmane Ok, I've made some progress and may need your help again with decyphering and fixing the remaining errors. Can matplotlib be used in a test or are there some restrictions I should be mindful of?

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Thanks @juntyr!

Cartopy often downloads shape files the first time that the user requires them during plotting, which is only supported if the pyodide-http patches are applied. How should this be handled?

I think using pyodide-http makes sense.

Can matplotlib be used in a test or are there some restrictions I should be mindful of?

You can see matplotlib tests for a reference when writing tests that use matplotlib.
You'll need to add some tests that check if the images (plots) are correctly created in Pyodide.

For example, you can run the code that generates an image (plot) locally, save the image into a file, then compare it with the same image created from Pyodide. You can copy the helper functions from matplotlib tests.

conftest.py:202: in pytest_runtest_call
    yield from extra_checks_test_wrapper(browser, trace_hiwire_refs, trace_pyproxies)
conftest.py:233: in extra_checks_test_wrapper
    assert (delta_proxies, delta_keys) == (0, 0) or delta_keys < 0
E   assert ((16, 34) == (0, 0)

You can skip this pyproxy check by adding the following decorators (again, check matplotlib tests to see how these decorators are applied). The error means there are some leaked Python objects after the test and it is quite normal when testing a package that interacts with JS.

pytest.mark.skip_refcount_check
pytest.mark.skip_pyproxy_check

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Thanks. @juntyr! Please update the changelog.

packages/pyshp/test_pyshp.py Outdated Show resolved Hide resolved
@ryanking13 ryanking13 merged commit e3789b8 into pyodide:main Jun 14, 2023
36 of 37 checks passed
@ryanking13
Copy link
Member

Thanks again @juntyr!

@juntyr juntyr deleted the cartopy branch June 14, 2023 06:00
@juntyr
Copy link
Contributor Author

juntyr commented Jun 14, 2023

Thanks for the help and reviews @ryanking13 and @hoodmane!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants