Skip to content

WMS options #348

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 12, 2016
Merged

WMS options #348

merged 2 commits into from
Feb 12, 2016

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Feb 10, 2016

@BibMartin
Copy link
Contributor

Happy to see the PR back.

In my mind, we were about to release v0.2 ; do you want to incorporate this PR in it ?

@ocefpaf
Copy link
Member Author

ocefpaf commented Feb 11, 2016

Yep. I will finish this today and focus on the release. I don't want to delay it more, but I meed WMS styles there for my own selfish reasons 😁

@ocefpaf
Copy link
Member Author

ocefpaf commented Feb 11, 2016

@BibMartin I removed the legacy WMS method test. That test was already a little flaky and we will remove method in after v0.2.0. However, I did not add a proper unittest for the new options.

The current test checks if the HTML is rendered and for the existence of bounds. The bounds are None now but I have a plan to use OWSLib to feed that information there. OWSLib will be an option dependency and I will leave that implementation to v0.3.0.

Can you review and merge this one?

BibMartin
BibMartin reviewed Feb 12, 2016
View reviewed changes
folium/features.py

self.version = version
# FIXME: Should be map CRS!
# self.crs = crs if crs else 'null
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that you would like to use this._parent.crs in the template to set the CRS ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I understood leaflet correct the WMS service might be in a different crs than the map and this variable is used to re-project it to the base map.

@BibMartin
Copy link
Contributor

OK, go!

BibMartin added a commit that referenced this pull request Feb 12, 2016
@BibMartin
Merge pull request #348 from ocefpaf/wms_docs_option
071126a 
WMS options
@BibMartin BibMartin merged commit 071126a into python-visualization:master Feb 12, 2016
@BibMartin BibMartin mentioned this pull request Feb 12, 2016
Merged
@ocefpaf ocefpaf deleted the wms_docs_option branch February 12, 2016 16:28
@ocefpaf
Copy link
Member Author

ocefpaf commented Feb 12, 2016

Thanks @BibMartin! Preparing the release now...

@BibMartin
Copy link
Contributor

Don't you want to finish #352 before ?

@ocefpaf
Copy link
Member Author

ocefpaf commented Feb 12, 2016

Working on that too.

@ocefpaf ocefpaf added the enhancement Feature request or idea about how to make folium better label Feb 12, 2016
@ocefpaf ocefpaf added this to the v0.2.0 milestone Feb 12, 2016
@ocefpaf ocefpaf mentioned this pull request Feb 12, 2016
Merged
sanga pushed a commit to sanga/folium that referenced this pull request Oct 24, 2017
@BibMartin
Merge pull request python-visualization#348 from ocefpaf/wms_docs_option
63fa492 
WMS options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement Feature request or idea about how to make folium better
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ocefpaf @BibMartin