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

RFC: Interpolate Highways #30

Merged
merged 1 commit into from
Sep 21, 2014
Merged

RFC: Interpolate Highways #30

merged 1 commit into from
Sep 21, 2014

Conversation

garborg
Copy link
Collaborator

@garborg garborg commented Sep 21, 2014

Closes #29.

@tedsteiner This needs to be rebased once #26 is merged -- I'll take care of that then. Also, I'm sure I'll work on other functionality, and it would be easier if master kept up (apart from wanting to push any fixes to METADATA so other users can take advantage) -- I appreciate that you're busy and the package isn't part of your current work -- perhaps you could give me a commit bit?

@tedsteiner
Copy link
Owner

Thank you for cleaning up the cropping functions, I knew they were messy but never made it back around to fixing it. I actually use this package on a day-to-day basis, but it does what I need it to already. I am in a bit of a deadline crunch right now, so these updates have been put on the back burner for me and I appreciate you looking into them.

I'm fine with giving you commit access if you plan to make a lot of changes, what functionality are you looking for? I think the code needs a little cleaning, but I would like to get to version 1.0 soon to indicate to potential users that the code is pretty stable, so comments on the interfaces are very welcome, too.

@tedsteiner
Copy link
Owner

I've given you repository access, so you're welcome to rebase this and get it into master.

As far as versioning goes (and pushing to Metadata.jl), I'd like to handle major/minor versioning for the time being, but I'm fine with patch versions as long as they pass the tests. My rule for minor versions has been that all added features/functions should have associated tests and documentation that go with them. I track progress toward the next major/minor version as Milestones here on GitHub, and I like to have new features associated with issues mainly just to make it easier to track what happened between versions. Does that sound reasonable?

I think everything is ready for version 0.6 except for adding some documentation for the streetmap simulation functions. I have a paper due in a few days, but I can try to get that updated soon (or you're welcome to add it, if you'd like).

Thanks a lot for your help!

@garborg
Copy link
Collaborator Author

garborg commented Sep 21, 2014

We can definitely put off commit access for now -- I mostly suggested it to help keep things moving if you weren't going to have time to review PRs and could use an extra set of eyes.

Regarding functionality, I'm going to try alternate routing algorithms (soon if I'm lucky), and I plan to look for for any other performance bottlenecks that come up for parsing and for routing use cases I run into, but I don't have any plans beside that, other than reporting any corner cases or issues I come across along the way.

P.S. Thanks again for putting this package out there -- I'm having a lot of fun with it 😄

@garborg
Copy link
Collaborator Author

garborg commented Sep 21, 2014

Oh, just saw your second message -- I'll definitely follow your lead on versioning strategy, feature tracking, etc.

@tedsteiner
Copy link
Owner

Good! I'm glad to hear it's working for you!

Alternate routing algorithms would be cool to see. And I'd love to be able to somehow use more accurate speed limits, right now I've just got it set so that every road in the same class has the same speed. I've also thought about making it so that you can import an elevation map to get more accurate routing by accounting for hills.

Cropping was leaving deleting the nodes, but leaving their keys in
highway node lists, which was breaking anything that needed
`node[highway.node[x]]`.

Tests files were also isolated in modules as the added tests behaved
differently when the file was run in isolation, and other existing
test files couldn't be run in isolation.
@garborg
Copy link
Collaborator Author

garborg commented Sep 21, 2014

That would be very cool!

This PR passed again after rebasing -- I'll merge it now.

garborg added a commit that referenced this pull request Sep 21, 2014
RFC: Interpolate Highways
@garborg garborg merged commit 05bfbc7 into tedsteiner:master Sep 21, 2014
@garborg garborg deleted the cropping branch September 21, 2014 19:54
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.

Highways aren't cropped/interpolated when middle nodes are out of bounds
2 participants