search

yolothreat / utilitybelt

A Python library for being a CND Batman....
MIT License
35 stars 12 forks source link

ip_to_geojson does not return JSON #56

Open blackfist opened 9 years ago

blackfist commented 9 years ago

WTH?

krmaxwell commented 9 years ago

I welcome your PR.

jeffbryner commented 9 years ago

Apparently it once did: https://github.com/yolothreat/utilitybelt/commit/5b749c2600a0777055718374e8f3314500620f96

blackfist commented 9 years ago

So we need to ask @sroberts why that was changed.

sroberts commented 9 years ago

So this is something I've been back and forth on. The idea of Utilitybelt is a Python library used within a other python apps (primarily). So if it's being called as a library doesn't it make more sense to respond with a Python native data structure rather than an intermediate format? I found myself writing a tool in Python, that called UB, got back JSON, and parsed the JSON back into a dict. It seemed unnecessary.

jeffbryner commented 9 years ago

Agreed, but if it says 'Generate GeoJSON for given IP address' it should return json.

Maybe the 'right' way is to have two functions called by a wrapper..one returns a dict, one json

krmaxwell commented 9 years ago

I like @jeffbryner 's suggestion, so that we're unambiguous about the returned data.

sroberts commented 9 years ago

I couldn't agree more. There are certainly use cases for both, they just need to be explicit. I dig it. :+1:

krmaxwell commented 9 years ago

that sounds suspiciously like "we welcome your PR" :D :D :D

On Thu, May 21, 2015 at 1:46 PM Scott J. Roberts notifications@github.com wrote:

I couldn't agree more. There are certainly use cases for both, they just need to be explicit. I dig it. [image: :+1:]

— Reply to this email directly or view it on GitHub https://github.com/yolothreat/utilitybelt/issues/56#issuecomment-104384715 .