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

chore: Upgrade v8.10 to math.gl@4.0.0. Remove gl-matrix #8200

Merged
merged 8 commits into from
Oct 23, 2023
Merged

Conversation

ibgreen
Copy link
Collaborator

@ibgreen ibgreen commented Oct 13, 2023

Background

  • loaders.gl 4.0 is needed on 8.10 for various early product work
  • loaders.gl 4.0 depends on math.gl 4.0 so to avoid having two versions of math.gl, let's bump math.gl in deck as well.

Change List

  • Bump to math.gl 4.0

@ibgreen ibgreen changed the title chore: Upgrade to math.gl@4.0.0-beta.1. Remove gl-matrix chore: Upgrade v8.10 to math.gl@4.0.0-beta.1. Remove gl-matrix Oct 14, 2023
@ibgreen ibgreen changed the title chore: Upgrade v8.10 to math.gl@4.0.0-beta.1. Remove gl-matrix chore: Upgrade v8.10 to math.gl@4.0.0. Remove gl-matrix Oct 20, 2023
@ibgreen ibgreen marked this pull request as ready for review October 23, 2023 14:22
@@ -22,7 +22,7 @@ import TEST_CASES from './test-cases';
import {WIDTH, HEIGHT} from './constants';
import {SnapshotTestRunner} from '@deck.gl/test-utils';

import './jupyter-widget';
// import './jupyter-widget';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Pessimistress @ajduberstein

As far as I can tell, math.gl 4.0 breaks jupyter-widget tests. The tests just freeze, I haven't figured out how to proceed with debugging

Copy link
Collaborator

@Pessimistress Pessimistress Oct 23, 2023

Choose a reason for hiding this comment

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

Please add a TODO comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The good news is that these tests seem to work again on 9.0, so it is presumably related to ES / CJS incompatibility causing some problems for Jupyter on 8.10.

We may need to make a decision on whether we can land this on 8.10 and temporarily break jupyter support). Since 8.10 is only dev releases and not for production this may be acceptable.

(I understand there is a new 8.0 python widgets library, maybe it addresses some of these things.)

@Pessimistress
Copy link
Collaborator

Why are we bumping major dependency versions in a minor release? luma.gl is still depending on older math.

"@math.gl/web-mercator": "^4.0.0",
"@math.gl/culling": "^4.0.0",
"@math.gl/geospatial": "^4.0.0",
"@math.gl/proj4": "^4.0.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are not going to help end users.

@ibgreen
Copy link
Collaborator Author

ibgreen commented Oct 23, 2023

Why are we bumping major dependency versions in a minor release?

loaders.gl is needed and is bringing in math.gl 4.0 anyways.

luma.gl is still depending on older math.

Yes. I was experimenting with making an 8.6 or 8.10 release of luma.gl to bump math.gl but the publish scripts are broken on my machine.

We can try to leave math.gl untouched on 8.10 for now and see if things work well with loaders.gl 4.0.

@ibgreen ibgreen merged commit 453a40f into 8.10-release Oct 23, 2023
2 checks passed
@ibgreen ibgreen deleted the ib/math4 branch October 23, 2023 20:21
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.

2 participants