-
Notifications
You must be signed in to change notification settings - Fork 5
Fixed open_browser issue and added functionality for map and config file #6
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
base: master
Are you sure you want to change the base?
Changes from all commits
fdd2922
b537073
e55d8e4
cf87092
873f7fb
94fa57a
2430f27
ac6b6e1
2bf44df
fb7bf39
8b8f9f2
404bd13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -40,14 +40,21 @@ def __init__( | |||||
| names=None, | ||||||
| read_only=False, | ||||||
| api_key=None, | ||||||
| style=None): | ||||||
| style=None, | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 8 params is too many to allow to be positional imo. I'd rather not change the existing API but we should add |
||||||
| config_file=None, | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO this should accept a config as a python dict, not as a path to a file |
||||||
| output_map=None, | ||||||
| open_browser=False): | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally didn't intend |
||||||
| """Visualize data using kepler.gl | ||||||
|
|
||||||
| Args: | ||||||
| data Optional[Union[List[]]]: | ||||||
| either None, a List of data objects, or a single data object. If | ||||||
| data is not None, then Visualize(data) will perform all steps, | ||||||
| including rendering and opening a browser. | ||||||
| `config_file` provides the path of config file. | ||||||
| `output_map` provides the location html file, if none then will | ||||||
| be dumped to temporaty files. | ||||||
| `open_browser` enables the browser opening if data is provided. | ||||||
|
Comment on lines
+54
to
+57
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These params should be in the same format as |
||||||
| """ | ||||||
| super(Visualize, self).__init__() | ||||||
|
|
||||||
|
|
@@ -59,23 +66,29 @@ def __init__( | |||||
| msg += 'environment variable not set.\nMap may not display.' | ||||||
| if self.MAPBOX_API_KEY is None: | ||||||
| print(msg) | ||||||
|
|
||||||
| if config_file is None: | ||||||
| self.config_file = resource_filename('keplergl_cli', 'keplergl_config.json') | ||||||
| else: | ||||||
| self.config_file = config_file | ||||||
| if output_map is not None: | ||||||
| self.path = output_map+'_vis.html' | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the user passes in a specific path, we shouldn't change it |
||||||
| else: | ||||||
| self.path = os.path.join(tempfile.mkdtemp(), 'defaultmap_vis.html') | ||||||
| config = self.config(style=style) | ||||||
| self.map = KeplerGl(config=config) | ||||||
|
|
||||||
| if data is not None: | ||||||
| self.add_data(data=data, names=names) | ||||||
| self.html_path = self.render(read_only=read_only) | ||||||
| self.html_path = self.render(read_only=read_only,open_browser=open_browser) | ||||||
|
|
||||||
| def config(self, style=None): | ||||||
| """Load kepler.gl config and insert Mapbox API Key""" | ||||||
|
|
||||||
| config_file = resource_filename( | ||||||
| 'keplergl_cli', 'keplergl_config.json') | ||||||
| # config_file = resource_filename('keplergl_cli', 'keplergl_config.json') | ||||||
|
|
||||||
| # First load config file as string, replace {MAPBOX_API_KEY} with the | ||||||
| # actual api key, then parse as JSON | ||||||
| with open(config_file) as f: | ||||||
| with open(self.config_file) as f: | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function shouldn't touch a user-provided config file. |
||||||
| text = f.read() | ||||||
|
|
||||||
| text = text.replace('{MAPBOX_API_KEY}', self.MAPBOX_API_KEY) | ||||||
|
|
@@ -143,14 +156,10 @@ def add_data(self, data, names=None): | |||||
| self.map.add_data(data=datum, name=name) | ||||||
|
|
||||||
| def render(self, open_browser=True, read_only=False, center_map=True): | ||||||
| """Export kepler.gl map to HTML file and open in Chrome | ||||||
| """Export kepler.gl map to HTML file and open in defauly system browser | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| """ | ||||||
| # Generate path to a temporary file | ||||||
| path = os.path.join(tempfile.mkdtemp(), 'vis.html') | ||||||
| self.map.save_to_html(file_name=path, read_only=read_only, center_map=center_map) | ||||||
|
|
||||||
| self.map.save_to_html(file_name=self.path, read_only=read_only, center_map=center_map) | ||||||
| # Open saved HTML file in new tab in default browser | ||||||
| if open_browser: | ||||||
| webbrowser.open_new_tab('file://' + path) | ||||||
|
|
||||||
| return path | ||||||
| webbrowser.open_new_tab('file://' + self.path) | ||||||
| return self.path | ||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: here and below are missing a newline at the end of the file |
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,4 +104,4 @@ | |
| "app": "kepler.gl", | ||
| "created_at": "Tue Oct 29 2019 17:15:02 GMT+0100 (Central European Standard Time)" | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,6 @@ | |
| test_suite='tests', | ||
| tests_require=test_requirements, | ||
| url='https://github.com/kylebarron/keplergl_cli', | ||
| version='0.3.3', | ||
| version='0.3.4', | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You shouldn't change the version here; we'll have a separate PR to bump the version, which will probably be 0.4.0 |
||
| zip_safe=False, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See below for discussion of whether
open_browsershould exist as a param to__init__, but if it does exist then it should be documented here