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

Add parsing of version and user data #7

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jarus
Copy link

@jarus jarus commented Jun 20, 2013

This pull request implements parsing of version and user data from XML source.

@olt
Copy link
Member

olt commented Jun 20, 2013

Thanks for providing a patch for #6.
Unfortunately this is a backwards incompatible change. Can you update it so that it only parses and passes the metadata when the parser was called with with_metadata=True?
I would also suggest to create a namedtuple for the metadata (including user_name and user_id):
http://docs.python.org/2/library/collections.html#collections.namedtuple

@jarus
Copy link
Author

jarus commented Jun 20, 2013

To use a namedtuple looks much better than a normal tuple. Thanks for your suggestion.

@jarus
Copy link
Author

jarus commented Jun 20, 2013

The parser could also convert the timestamp to a datetime object. I would say there is no performance issue:

$ python -m timeit -s 'from datetime import datetime' 'datetime.strptime("2013-06-04T14:35:21Z", "%Y-%m-%dT%H:%M:%SZ")'
10000 loops, best of 3: 23.8 usec per loop

@olt
Copy link
Member

olt commented Jun 20, 2013

"only" 23.8usec, but for each node, way and relation. For Germany alone just parsing the datetime would take ~30min.
I would leave it out, maybe add it as code snippet in the documentation.
If you really need it in the code, make sure it is only called when with_metadata is true.

@jarus
Copy link
Author

jarus commented Jun 20, 2013

Okay, additional 30min for a datetime object is a little bit to high.

@emacsen
Copy link

emacsen commented Aug 22, 2013

Any news on this patch? This would be very helpful to me.

For the record- I agree with not turning the timestamp metadata into a datetime object, since this can easily be done by the programmer (perhaps include an example to illustrate how).

The data I need is the version of the object, and the changeset.

@jarus
Copy link
Author

jarus commented Aug 23, 2013

It would be nice that the with_metadata kwarg would be also supported by the pdb parser.

In my eyes the implementation for the osm file is complete and works well.

@emacsen
Copy link

emacsen commented Aug 23, 2013

I'll take a look and see what I can do. My git-foo is not so strong as to easily know how to grab this request and merge it into my branch before working on it

@sabas
Copy link

sabas commented Aug 29, 2014

Was this part released?

@jarus
Copy link
Author

jarus commented Aug 29, 2014

No it wasn't released yet. The main point was that the pdb parser doesn't support the extraction of meta data which provide this patch for osm files.

On 29.08.2014, at 22:07, sabas [email protected] wrote:

Was this part released?


Reply to this email directly or view it on GitHub.

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.

4 participants