-
Notifications
You must be signed in to change notification settings - Fork 827
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
Dictionaries attributes not handled in io.load_graphml() #1067
Comments
To save everything to GraphML, OSMnx stringifies the attribute values. When it reloads the GraphML, the For example, if you wanted to have a floating point type for some custom attribute, you'd specify However, it works differently for a dictionary, because of how Python works. That is, So, in your code example, the converters you're looking for would be something like: import ast
ox.load_graphml(filepath = 'testsave.graphml',
node_dtypes = {'test_dict_attr': ast.literal_eval},
edge_dtypes = {'test_dict_attr': ast.literal_eval}) |
Exactly, that's the workaround that I'm using, that I referred to. For my purposes (custom attributes for processing metrics) it works fine. I thought I should raise the issue, given that the description for load_graphml doesn't limit support to a specific subset of basic dtypes, so a user could assume that all should work. I could readily imagine you might want to restrict code support to dtypes that exist in OSM, for example, if that would exclude dict, and I assume that that is why you have coded support for bool and list dtypes. Totally up to you, of course, how to approach this question, whether to extend code or just to document. Regards |
That is the current logic, yes, in that we handle types that exist in OSM plus types we need to make OSMnx's graph building work (ie, Python lists to handle simplified edges' attributes). That said, it has always been a bit confusing regarding when a user needs to use It should be fairly simple to add this by just expanding the logic that already exists in the |
Geoff, I've opened a draft PR with proposed changes. This is my first 'real-world' PR so my apologies if I've gotten things messed up. I wasn't sure how far to get into the hooks and test runs as described in the contribution guideline, but I am assuming that is if I were to be actually executing the merge (which I certainly don't think I'm to do?). I did test the changes by installing from my github into an env and running simple tests of the sort I described here above. |
Closed by #1075 |
Contributing guidelines
Documentation
Existing issues
What operating system and Python version are you using?
Win11, Python 3.10.10
What OSMnx version are you using?
1.6.0
Environment packages and versions
How did you install OSMnx?
Conda and conda-forge
Problem description
Hello Geoff,
Similar to issue #665 for bool types, _convert_node_attr_types() and _convert_edge_attr_types() can't interpret attributes of type dict stored as strings in a graphml object being loaded using load_graphml(). It throws a "ValueError: dictionary update sequence element #0 has length 1;...", when type 'dict' is provided in node_dtypes or edge_dtypes, e.g. {'amenity_counts': dict} for a custom attribute 'amenity_counts' of type dict.
_convert_edge_attr_types() already has code to handle lists, checking for '[' and ']' characters then using ast.literal_eval(), and it appears that a similar addition could be made for both functions that also included checks for '{' and '}'.
I am successfully using the workaround you suggested in #665, to use dtype ast.eval_literal instead of dtype dict for these attributes.
Thanks
Neil
Complete minimal reproducible example
The text was updated successfully, but these errors were encountered: