-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Satellite geo projection #4781
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
Comments
Hi @disberd! Yes, we'd be happy to accept a PR to include this projection. I think you've covered all or nearly all the changes that will be required, but from the looks of it plotly.js/src/plots/geo/projections.js Lines 10 to 12 in 22dd9c1
Which makes it look as though all that should be needed for that part of the update is to run
Then as a final step the new projection would want to be included in one or two test images - but we can help once it gets to that point. |
Hi @alexcjohnson! Thanks for the reply. I will start trying modifying the code as mentioned originally and check if I manage to build plotly with the modification to test functionality while we wait from a feedback from @etpinard. I'll ask for help with generating the test images when I manage to get some results! |
I started working on the issue and applying the modification to projections and constants as suggested in my first post does indeed create the correct projection. Now there are some other few issued that arised and should be fixed to make the projection work as intended. For the moment there are 3 that I identfied:
@@ -671,9 +671,13 @@ function getProjection(geoLayout) {
var projection = d3.geo[constants.projNames[projType]]();
- var clipAngle = geoLayout._isClipped ?
- constants.lonaxisSpan[projType] / 2 :
- null;
+ if (projType == "satellite") {
+ clipAngle = (Math.acos(1 / projection.distance()) * 180) / Math.PI;
+ } else {
+ var clipAngle = geoLayout._isClipped
+ ? constants.lonaxisSpan[projType] / 2
+ : null;
+ } The change correctly handles the clipAngle but I am not sure whether this is the best way it could be done or if it goes against some coding guidelines here.
While waiting for some feedback, I'll keep try and finding an answer to point 2 and 3. |
Upgrading to use |
Cool to see that with the plotly update this kind of projection (together with others) might be implemented soon. |
Hello,
I recently started using plotly.js and would like to congratulate with the amazing works that had gone into this nice plotting tool.
In my work I often have to deal with plots involving earth as viewed from a satellite, for which the general perspective projection would be very useful to have among the already available ones (it is basically an ortographic projection from an arbitrary distance instead of an infinite one).
I am relatively new to javascript but by looking at the code of plotly.js, given that projections seems to be handeld by the d3-geo package, it seems that such addition would be quite straightforward to include.
Specifically, based on how the other projections seem to be implemented, it seems it would be sufficient to add the new projection to src\plots\geo\projections.js according to the code in this page of the d3-geo-projections repo.
This is the code I tried to modify from the repo to fit the style and definitions found in the projections.js file (not so sure about the scale and clipAngle children of p at the end of this code as they don't seem to be defined in other projections in plotly.js)
And make the projection selectable by appending its name at the end of params.projNames in the file src\plots\geo\constants.js
Is this all that is needed to implement a new projection or are there other parts of the code that might be impacted that I did not realize?
Provided this is really all that is necessary, I will try to implement and test these changes myself if needed in the next few days but having never used node or really coded in javascript before, it might take a while before I am eventually able to test and learn how to do a pull request.
The text was updated successfully, but these errors were encountered: