Dictionaries attributes not handled in io.load_graphml() about osmnx HOT 5 CLOSED

I could readily imagine you might want to restrict code support to dtypes that exist in OSM

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 literal_eval. It seems like handling dicts/sets (ie, any attribute value that starts with { and ends with }) might be a graceful compromise solution between simplicity and predictability.

It should be fairly simple to add this by just expanding the logic that already exists in the io module to handle list conversion. If you'd like to take a quick stab at a PR, I could take a look.

from osmnx.

To save everything to GraphML, OSMnx stringifies the attribute values. When it reloads the GraphML, the node_dtypes argument (and its peers) are used to determine how to convert each stringified attribute value back to its correct, original type.

For example, if you wanted to have a floating point type for some custom attribute, you'd specify node_dtypes = {"custom_attr": float} because float("123.45") will convert that string value to a float value.

However, it works differently for a dictionary, because of how Python works. That is, dict("{'foo':'bar'}") will not produce a dictionary, but rather the ValueError you noted above. Instead, to convert a string-representation of a dictionary to a dictionary, you would run ast.literal_eval("{'foo':'bar'}").

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})

from osmnx.

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
Neil

from osmnx.

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.
All feedback on what I should have done differently most welcome...
Regards
Neil

from osmnx.

Closed by #1075

from osmnx.