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

[0.2.2.dev1] New MergedChoiceTable feature: from_df() construction #67

Merged
merged 11 commits into from
Apr 14, 2020

Conversation

mxndrwgrdnr
Copy link
Member

This is a feature we've wanted for a long time, the ability to create a MergedChoiceTable object from a dataframe. This gives the user the ability to create an mct using the standard choicemodels workflow, but to then extract it, manipulate it, add new terms, and cast it back to an object that can be used by urbansim_templates or choicemodels once again.

My goal was to make as few changes as possible to the core class structure, and as a result my solution is not the most elegant. It relies on the instantiation of two empty dataframes and a wrongly typed class attribute to satisfy what are currently required parameters of the MergedChoiceTable class. I'm open to suggestions if we think there's a better way to achieve the intended result.

@coveralls
Copy link

coveralls commented Apr 13, 2020

Coverage Status

Coverage increased (+0.3%) to 76.42% when pulling f1ec684 on mnl_w_mct_df into 54c936d on master.

@smmaurer
Copy link
Member

smmaurer commented Apr 14, 2020

Thanks @mxndrwgrdnr! It will be great to finally have this. I completely agree with this approach, as we discussed in Slack.

Here are some things i added just now:

  • got Travis working (it was a statsmodels and HDF install issue)
  • adjusted the MCT constructor and properties a bit so that .from_df() works more smoothly
  • added some relevant unit tests
  • adjusted .from_df() accordingly
  • added a note about row ordering to the docstring
  • updated versioning and changelog

If this all looks good, go ahead and merge!

@mxndrwgrdnr mxndrwgrdnr merged commit b173353 into master Apr 14, 2020
@mxndrwgrdnr
Copy link
Member Author

Looks great, Sam. Thanks for getting it across the finish line.

@mxndrwgrdnr mxndrwgrdnr deleted the mnl_w_mct_df branch April 14, 2020 15:19
@smmaurer smmaurer changed the title New MergedChoiceTable feature: from_df() construction [0.2.2.dev1] New MergedChoiceTable feature: from_df() construction Apr 17, 2020
@smmaurer smmaurer mentioned this pull request Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants