search

ActivitySim / activitysim

An Open Platform for Activity-Based Travel Modeling
https://activitysim.github.io
BSD 3-Clause "New" or "Revised" License
189 stars 96 forks source link

a lot of PRs #12

Closed fscottfoti closed 9 years ago

fscottfoti commented 9 years ago

As people may have noticed, there are a lot of pull requests right now (more than I would like). I'd love to start merging the early PRs, but we're waiting on a resolution to the omx issue. Right now all these PRs depend on the omx that we have within activitysim. If we want to depend on omx outside of activitysim then we need to make those changes in omx first. Or alternatively, I suppose we could leave omx in activitysim for now and then remove it once the omx repo is setup so that it can be a dependency of activitysim. Bottom line is I will start merging PRs if people are ok with omx being in activitysim for now. Any takers?

waddell commented 9 years ago

I thought the resolution on this issue was that Billy would make changes to omx in its own repository? Possibly with patches or pull requests from us?

On Tue, Jan 13, 2015 at 12:32 PM, Fletcher Foti notifications@github.com wrote:

As people may have noticed, there are a lot of pull requests right now (more than I would like). I'd love to start merging the early PRs, but we're waiting on a resolution to the omx issue. Right now all these PRs depend on the omx that we have within activitysim. If we want to depend on omx outside of activitysim then we need to make those changes in omx first. Or alternatively, I suppose we could leave omx in activitysim for now and then remove it once the omx repo is setup so that it can be a dependency of activitysim. Bottom line is I will start merging PRs if people are ok with omx being in activitysim for now. Any takers?

— Reply to this email directly or view it on GitHub https://github.com/synthicity/activitysim/issues/12.

fscottfoti commented 9 years ago

That is the resolution. I wanted to alert everyone that we're waiting to merge these PRs until that is completed - that having this many PRs is not ideal.

waddell commented 9 years ago

Got it. Do we need to send a pull request on the omx repo to move this along?

On Tue, Jan 13, 2015 at 12:44 PM, Fletcher Foti notifications@github.com wrote:

That is the resolution. I wanted to alert everyone that we're waiting to merge these PRs until that is completed - that having this many PRs is not ideal.

— Reply to this email directly or view it on GitHub https://github.com/synthicity/activitysim/issues/12#issuecomment-69816271 .

fscottfoti commented 9 years ago

I think we have an open issue there - not sure what the latest is. @jiffyclub?

jiffyclub commented 9 years ago

I opened osPlanning/omx#13 last month, but haven't heard anything.

danielsclint commented 9 years ago

Let me try to connect with @billyc and @bstabler directly today. I would prefer to try and keep these two library separate in order to avoid many of the current issues that plagued the PB-Commons library over the years. However, if we don't get a commitment by tomorrow to make adjustments to OMX, I would be fine with having a copy here FOR NOW.

danielsclint commented 9 years ago

I was able to connect with @billyc and @bstabler. They are both in Washington for TRB this week. But, @billyc said he could take care of this on the OMX side starting next week. Is it feasible to hold onto the PR requests until then or would it make more sense to merge the PRs and then make the transition when OMX ready in the next week or two?

fscottfoti commented 9 years ago

I'm happy to wait on the updates - just wanted to keep everyone in the loop. No hurry and enjoy TRB @billyc

fscottfoti commented 9 years ago

Finally circling back around to this... any word on the latest on the OMX integration? Can we help get OMX distributable as a Python package in the usual way (so that we can import it)?

danielsclint commented 9 years ago

From an email from @billyc...

I don't remember if you're on the OMX mailing list – see below, PIP installer is done.

Sent: Thursday, January 15, 2015 1:03 PM Subject: pip install openmatrix -- Python installer created.

I've created a standard pip installer for the python OMX api. You can now install omx into your python distribution with the following magic command:

pip install openmatrix

This command will fetch openmatrix from the PyPi repository and download/install it for you.

IMPORTANT NOTE! The package name "omx" was already taken on pip for a lame xml library that no one uses. Thus our little project goes by "openmatrix" on pip instead of "omx".

This means your import statements need to change. Any occurrence in your code of:

import omx

must be changed to:

import openmatrix as omx

Other than that I think we're good to go. ..b

fscottfoti commented 9 years ago

Sweet. @jiffyclub does this all look right to you? We should start cascading this update in the PRs and start merging...

jiffyclub commented 9 years ago

Definitely give it a try. I made some minor changes when I brought omx into activitysim, but I doubt we are relying on them.