Skip to content
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

Create a more "pythonic" API #39

Open
metaodi opened this issue Jan 5, 2015 · 9 comments
Open

Create a more "pythonic" API #39

metaodi opened this issue Jan 5, 2015 · 9 comments

Comments

@metaodi
Copy link
Owner

metaodi commented Jan 5, 2015

The public API of osmapi is not very pythonic, it would be nice to create an alternative - more pythonic - API to be better integrated in the whole python world.

This starts with the camelcase naming of the methods, but also the lack of structure. I'm not yet sure how an ideal structure would look like, but maybe a classic OOP approach wouldn't be too bad, and then having a class for each type of OSM object.

But the current API should stay valid to not break existing code.

@austinhartzheim
Copy link
Contributor

I am somewhat interested in working on this. But, I am wondering how you plan to remove the camel case without breaking existing code?

Some enhancements can be done within the current framework. For example, one might subclass dict as follows:

class Node(dict):

    def history(self):
        return osmapi.NodeHistory(self['id'])

    def relations(self):
        return osmapi.NodeRelations(self['id'])

In this case, the return line of NodeGet would become:

return Node(self._DomParseNode(data))

This is probably a rare case, but code depending on using type(node) might break. Also, it is still somewhat strange to use camel case here, but it is slightly better because it is returning a custom class instance.

Personally, I think it is better to create a new API. The current code looks easy enough to maintain if bugs are filed (and I can help maintain it). However, I am also unaware of what projects are currently using this API and how much work they would need to invest to update to a new API.

@metaodi
Copy link
Owner Author

metaodi commented May 29, 2015

@austinhartzheim I'm not yet sure how to approach this. But I guess you're on the right track. I'm not really afraid of a BC-break (maybe the new API would be major release 1.0), as the old code is quite stable and no one is forced to upgrade. But as this is kind of the go-to-package for python, it really annoys me, that it's not more "pythonic".

I'm planning a refactoring to split the code in files and remove all the HTTP code and replace it with requests. I wouldn't start this issue before that. But we might use this issue to figure out together what a good "pythonic" API could look like.

@metaodi
Copy link
Owner Author

metaodi commented Feb 5, 2016

Some notes based on Python Summit talk about API Design:

  • Use verbs for methods, nouns for properties
  • Use pythonic naming conventions (get rid of Java-style naming of methods)
  • Split module by OSM type
  • Try to keep parts of the old API if possible
  • Ease the pain by issuing deprecation warning warnings.warn("...", DeprecationWarning)

@joeblankenship1
Copy link

I would love to contribute to this. Is there a plan in place to start restructuring the API? If the above points are the priority, I could start there.

@metaodi
Copy link
Owner Author

metaodi commented Mar 3, 2016

@joeblankenship1 I'm in the middle of refactoring to start this process (basically splitting up the library in several files). I'd suggest to use this issue to discuss how a good API would look like. Maybe add references to good examples etc.

As soon as I'm ready, I'll create a PR with my suggestions and would then be happy to receive feedback from all of you to improve it until we feel comfortable to release it.

@joeblankenship1
Copy link

Sounds good; I'll start doing some research!

@metaodi
Copy link
Owner Author

metaodi commented Oct 12, 2017

I want to deprecate the whole changesetauto logic as it's a lot of magic and especially the use of the destructor __del__ to close a changeset on destruction is very flaky (i.e. it is not guarenteed, that this code is run, or when it is run). I opened #89 to introduce the more pythonic context managers.

This would lead to code like that (including the more pythonic snake_case for methods):

import osmapi
osm = osmapi.OsmApi(username = "metaodi", password = "*******")
with osm.open_changeset(comment="Create test node") as changeset_id:
     print "Opened changeset %s" % changeset_id
     osm.create_node({"lon": 1, "lat": 1, "tag": {}})

The context manager (using the with keyword) would take care of the changeset as a resource and close it automatically after the with block.

This is roughly the same as this code of the current code base:

import osmapi
osm = osmapi.OsmApi(username = "metaodi", password = "*******")
changeset_id = osm.ChangesetCreate({"comment": "Create test node"})
print "Opened changeset %s" % changeset_id
osm.NodeCreate({"lon": 1, "lat": 1, "tag": {}})
osm.ChangesetClose()

@b-jazz
Copy link

b-jazz commented Apr 27, 2019

But the current API should stay valid to not break existing code.

Just wanted to step in and remind everyone that the purpose of major release numbers (v2.0.0 vs v1.0.0) is to indicate changes that might break previous usage. I don't think you should be worried about breaking code. Just bump the major version and people that don't want to fix their code can lock into the v1.* versions.

@metaodi
Copy link
Owner Author

metaodi commented Apr 27, 2019

@b-jazz you're probably right 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants