-
Notifications
You must be signed in to change notification settings - Fork 9
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
Allow xml write/read of additional attributes #118
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…attributes for nodes, links and stops
mfitz
requested changes
Jun 14, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the README need updating to describe this new functionality?
tests/test_data/matsim/schedule_route_with_additional_attrib.xml
Outdated
Show resolved
Hide resolved
* add assuming java types for additional attributes when writing to xml * add assuming python types for additional java-typed attributes when reading from xml * fix lingering dependence on long form attributes * update notebooks with simple form attributes * extra fixes, add dtype param to road pricing to match with OSM ID dtypes * increase code coverage * address PR comments * PR update: enable forcing long form when reading matsim networks * lint
# Conflicts: # scripts/add_elevation_to_network.py # tests/test_core_network.py
mfitz
approved these changes
Jul 1, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to go!
# Conflicts: # genet/outputs_handler/matsim_xml_writer.py deleted in HEAD and modified in master.
* add example script for snapping pt stops to other parts of the network * warn of unsnapped stops if distance threshold reached prematurely * relax condition * add handling multiple network modes and teleport (access only) modes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Until now we only allowed additional attributes for links. They exist in the following format in the link attribute dictionaries:
and are saved to xml in the following format:
This PR extends this to other elements:
which follow the same format as links:
'attributes': {'additional_attrib': {'name': 'additional_attrib', 'class': 'java.lang.String', 'text': 'attrib_value'}}
, andExample
This is what a network with extra attributes looks like:
This is an example of a schedule with additional attributes:
Caveats
There are a few caveats, I think I cleaned it up so it makes sense, but if you have better ideas, do let me know!
C1
Attributes for Schedule elements (Routes, Services, Stops) can be changed using the dedicated
apply_attributes_to_X
methods in theSchedule
in which case you apply the changes in the same way as network nodes and links:and that is logged into the schedule's change_log, BUT, those elements can also be accessed and a class instance of e.g.
Route
will have anattributes
attribute that can be accessed and changed, but that will not be recorded in the changelog:Services, Routes and Stops may or may not have the
attributes
attribute and that can be checked via thex.has_attrib('attributes')
method.C2
Network
andSchedule
attributes can only be accessed via theattributes
attribute and that always exists and at minimum holds the projection for the element, and for most our networks will have the 'simplified True' attribute. Stuff can be added and changed, and it is not recordedC3
There is another element for MATSim that is not supported: trip departures can also have additional attributes. But the way we store information for trips does not make it easy for us to allow this. I doubt we'd make much use of it though.
Future Work
FW 1
Now that network and schedule elements always have a crs attribute we should make the
epsg
parameter in the matsim read functions optional. One thing that needs to happen also is for PUMA to always save these attributes too.FW 2 [SOLVED, see #124]
I absolutely hate this format:
We should implement something that has less repetition, at least:
or maybe even
which default to the right java class when you save it to matsim...