Skip to content

Use getattr instead of eval#207

Merged
giswqs merged 2 commits into
opengeos:masterfrom
Piphi5:use_get_attr
Feb 25, 2022
Merged

Use getattr instead of eval#207
giswqs merged 2 commits into
opengeos:masterfrom
Piphi5:use_get_attr

Conversation

@Piphi5

@Piphi5 Piphi5 commented Feb 25, 2022

Copy link
Copy Markdown
Contributor

getattr is more robust and cleaner than trying to use eval for getting parameters

@giswqs

giswqs commented Feb 25, 2022

Copy link
Copy Markdown
Member

It seems get_attr does not exist. Should it be getattr?

@Piphi5

Piphi5 commented Feb 25, 2022

Copy link
Copy Markdown
Contributor Author

Yeah it is, my bad.

@Piphi5 Piphi5 changed the title Use get_attr instead of eval Use getattr instead of eval Feb 25, 2022
@giswqs

giswqs commented Feb 25, 2022

Copy link
Copy Markdown
Member

It is still failing. I am not sure if they are caused by the tests only or getattr changes result in failed tests. Can you fix it?

@Piphi5

Piphi5 commented Feb 25, 2022

Copy link
Copy Markdown
Contributor Author

Yeah I'll look into it. It seems like trying to call "OpenStreetMap.Mapnik" as a single parameter may be the main issue

@lgtm-com

lgtm-com Bot commented Feb 25, 2022

Copy link
Copy Markdown

This pull request fixes 3 alerts when merging 98d74ee into c28a48a - view on LGTM.com

fixed alerts:

  • 3 for Suspicious unused loop iteration variable

@giswqs

giswqs commented Feb 25, 2022

Copy link
Copy Markdown
Member

It is improving. Only test_add_circle_markers_from_xy is failing.

@Piphi5

Piphi5 commented Feb 25, 2022

Copy link
Copy Markdown
Contributor Author

Is there any reason why this:

out_str = m.to_html()
assert "World Cities" in out_str

would fail with add_circle_markers_from_xy but not fail with add_xy_data?

@lgtm-com

lgtm-com Bot commented Feb 25, 2022

Copy link
Copy Markdown

This pull request fixes 3 alerts when merging 1b051a8 into d5d4248 - view on LGTM.com

fixed alerts:

  • 3 for Suspicious unused loop iteration variable

@giswqs

giswqs commented Feb 25, 2022

Copy link
Copy Markdown
Member

It should be name rather than layer_name. It seems add_circle_markers_from_xy is not producing the expected results. There is only one circle plotted on the map. It seems there is a bug with this function, but it might not be related to your commits.

import leafmap
import ipyleaflet

m = leafmap.Map()
in_csv = "https://raw.githubusercontent.com/giswqs/data/main/world/world_cities.csv"

m.add_circle_markers_from_xy(
    in_csv, x="longitude", y="latitude", name="World Cities"
)
m.add_control(ipyleaflet.LayersControl(position="topright"))
m

image

@Piphi5

Piphi5 commented Feb 25, 2022

Copy link
Copy Markdown
Contributor Author

is the add_circle bug reproducable without the code in this pr?

@giswqs

giswqs commented Feb 25, 2022

Copy link
Copy Markdown
Member

It is a bug of the ipyleaflet backend. Works fine with folium.

@giswqs
giswqs merged commit 84ad70c into opengeos:master Feb 25, 2022
@Piphi5
Piphi5 deleted the use_get_attr branch February 25, 2022 22:17
@lgtm-com

lgtm-com Bot commented Feb 25, 2022

Copy link
Copy Markdown

This pull request fixes 3 alerts when merging 05b6daa into d5d4248 - view on LGTM.com

fixed alerts:

  • 3 for Suspicious unused loop iteration variable

giswqs added a commit that referenced this pull request Feb 28, 2022
sthagen pushed a commit to sthagen/giswqs-leafmap that referenced this pull request Feb 18, 2023
Use getattr instead of eval

Former-commit-id: 84ad70c
sthagen pushed a commit to sthagen/giswqs-leafmap that referenced this pull request Feb 18, 2023
sthagen pushed a commit to sthagen/giswqs-leafmap that referenced this pull request Jul 10, 2024
Use getattr instead of eval

Former-commit-id: 7f51c9d
sthagen pushed a commit to sthagen/giswqs-leafmap that referenced this pull request Jul 10, 2024
kuzja111 pushed a commit to kuzja111/leafmap that referenced this pull request Jul 25, 2024
Use getattr instead of eval

Former-commit-id: 0ef5dae
kuzja111 pushed a commit to kuzja111/leafmap that referenced this pull request Jul 25, 2024
giswqs added a commit that referenced this pull request Dec 6, 2025
Use getattr instead of eval

Former-commit-id: e5f80249a5c17ee3abc264ced94e317f38740173
giswqs added a commit that referenced this pull request Dec 6, 2025
Former-commit-id: a5eaaa1aa907b7c8e75376d8f3b14e6cef6a5060
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