Support for multiple projects #323
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
|
First impression is this looks good, thanks! I do not know how this interacts with the QGIS integration, though. I will have a closer look and think about this when I get some time, it's pretty hectic at the moment so this could take a couple of days. |
|
Hey, @jamiecook . Can you take a look at this? It resembles a little some of the changes you did on polarislib, so you might be the best person to review it. |
jamiecook
reviewed
Jun 20, 2022
jamiecook
reviewed
Jun 20, 2022
jamiecook
reviewed
Jun 20, 2022
jamiecook
approved these changes
Jun 20, 2022
There was a problem hiding this comment.
My thoughts mirror @janzill's - this is really good and something that will help the overall architecture going forward.
To be clear - this doesn't fix all the problems with global variables but it does condense dramatically the global variables to a single place.
What I've been trying to do in another similar project is to move all of the global state logic into the QGIS plugin (where it's needed) and keep it out of the core library logic altogether. This is a great step in that direction.
Things I think need further consideration in a future PR:
- The level of coupling up and down the chain (networks have links and links have a network, networks have a project and a project has a network)
- Thats it
|
Thanks for the reviews, I've processed your comments and fixed a few things that were breaking the pipeline. If there are no additional comments, I think it's ready to be merged |
|
I am sorry for the delay, @elfjes . Me and my wife got pretty sick for a couple of weeks (still not 100% back) and everything went haywire on my end. |
|
No problem @pedrocamargo. Wish you a speedy recovery. Thanks for merging :) |
pedrocamargo
added a commit
that referenced
this pull request
Jul 24, 2022
* Updates shapely deprecated methods (#281) * Improves performance of triggers (#278) * documentation correction (#282) * INRETS and bpr2 VD functions (#273) * French INRETS VD function First implementation of the French INRETS Volume Delay function. Beta is not used but kept for better consistancy across VD functions. * BPR2 implementation First implementation of BPR2 Volume Delay function. This doubles beta above capacity so that fewer vehicles are affected when capacity is exceeded. Double Beta insteed of using a Beta' allow to use only 2 parameters as for other VD functions. Integration into QGIS GUI is also easier * Two small changes around the edges (#288) * Closes issue #289 and adds small feature * Fixes long-standing formatting issue * fixes traffic assignment tests (#293) * Follows the Wikimedia user-agent policy (#295) * Makes sure we follow the Wikimedia user agent policy https://meta.wikimedia.org/wiki/User-Agent_policy * Adresses issue #285 (#297) * Allows for skims to be set after the creation of traffic classes * Improvement on skim saving * Updates example with change in skim saving method * Fixes typo * Fixes creation of skim matrices on multi-class assignment * Fixes final skim for travel time and introduces basic testing * Fixes example with new skimming standards * cleans tests * Relocates and Fixes path saving tests * Updates tests * QGIS adjustments (#280) * Threading * Adds connection refresh capability * Stores binaries with all tests * Makes feather optional on import * Removes repeated code * Bumps up version * Adds doc version to index * Bumps version for new release * closes issue #303 * closes issue #303 (#304) * Fixes issue 307 * Improves database structure (#310) * Improves database structure * Improves database structure * numpy 1.22 is still not the default * CI workflow to generate binaries for different NumPy versions * Sets up better tests * Sets up better tests * numpy 1.22 is still not the default * Cleans test * Run black on all source files * Adding conda_forge_recipe * Support for multiple projects (#323) * Matrix from list (#325) * Pedro/new test fix (#322) * Speeding up OSM network import * only run actions that require secrets from the Aequilbrae account. (#329) * Support logging for multiple projects, log most warnings instead of raising them (#327) * one logger per project, log most warnings instead of raising them * Multiple changes * renamed starts_logging and combine with log.py * moved additional setup code to __init__.py * undo falsely blackened line * process feedback * cleanup ci setup for unittests - do errors still occur? * Small changes to the Delaunay line creation in trying to identify consistent breaking point Co-authored-by: Danilo Ebbinghaus <91709021+daniloebbinghaus@users.noreply.github.com> Co-authored-by: Arthur Evrard <83211842+Art-Ev@users.noreply.github.com> Co-authored-by: Jan Zill <jan.zill@veitchlister.com.au> Co-authored-by: Jan <jan.christoph.zill@gmail.com> Co-authored-by: Jamie Cook <jamie.cook@veitchlister.com.au> Co-authored-by: PelleK <elfjes@users.noreply.github.com> Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov>
pedrocamargo
added a commit
that referenced
this pull request
Nov 3, 2022
* Updates shapely deprecated methods (#281) * Improves performance of triggers (#278) * documentation correction (#282) * INRETS and bpr2 VD functions (#273) * French INRETS VD function First implementation of the French INRETS Volume Delay function. Beta is not used but kept for better consistancy across VD functions. * BPR2 implementation First implementation of BPR2 Volume Delay function. This doubles beta above capacity so that fewer vehicles are affected when capacity is exceeded. Double Beta instead of using a Beta' allow to use only 2 parameters as for other VD functions. Integration into QGIS GUI is also easier * Fixes styles to expose test failures * Fixes styles to expose test failures * reverts white space changes in bpr * Fixes inrets tests * Two small changes around the edges (#288) * Adds the obvious field vot to the modes table and creates and adds a script for compiling that may be useful during development * Update tests/compile.py Co-authored-by: Jamie Cook <jamie.cook@veitchlister.com.au> * Fixes import * Closes issue #289 and adds small feature * Fixes long-standing formatting issue * fixes traffic assignment tests (#293) Brilliant work, @janzill That original test was really out of whack!!! * Follows the Wikimedia user-agent policy (#295) * Makes sure we follow the Wikimedia user agent policy https://meta.wikimedia.org/wiki/User-Agent_policy * Adresses issue #285 (#297) * Allows for skims to be set after the creation of traffic classes * Improvement on skim saving * Updates example with change in skim saving method * Fixes typo * Fixes creation of skim matrices on multi-class assignment * Fixes final skim for travel time and introduces basic testing * Fixes example with new skimming standards * cleans tests * Relocates and Fixes path saving tests * Updates tests * QGIS adjustments (#280) * Threading * Adds connection refresh capability * Stores binaries with all tests * Makes feather optional on import * Removes repeated code * Bumps up version * Adds doc version to index * Bumps version for new release * closes issue #303 * closes issue #303 (#304) * Fixes issue 307 * Improves database structure (#310) * Improves database structure * Improves database structure * numpy 1.22 is still not the default * CI workflow to generate binaries for different NumPy versions * Sets up better tests * Sets up better tests * numpy 1.22 is still not the default * use temp dir * guessing this is a case problem * guessing this is a case problem * guessing this is a case problem * it was a case issue * Cleans test * Run black on all source files * Update build_linux.yml * Adding conda_forge_recipe * Support for multiple projects (#323) * Matrix from list (#325) * Pedro/new test fix (#322) * Fixes linux building * Fixes linux building * Speeding up OSM network import * only run actions that require secrets from the Aequilbrae account. (#329) * Support logging for multiple projects, log most warnings instead of raising them (#327) * one logger per project, log most warnings instead of raising them * Multiple changes * renamed starts_logging and combine with log.py * moved additional setup code to __init__.py * undo falsely blackened line * process feedback * cleanup ci setup for unittests - do errors still occur? * Edit the correct ci file * we do need more requirements * run in docker to improve isolation * apt get update Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> * Small changes to the Delaunay line creation in trying to identify consistent breaking point * Fix some warnings Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> * Refactor tests to pytest (#331) * Refactor tests * Cleanup and some comments * fix tests * process feedback * Update tests/aequilibrae/conftest.py Co-authored-by: Jamie Cook <jimi.cook@gmail.com> * rescope session-scoped fixtures inside a class to class scope to avoid confusion * fix trailing whitespace Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> Co-authored-by: Jamie Cook <jimi.cook@gmail.com> * change the check for secrets from repo_owner to has_secret (#333) * better deal with np.memmap open files (#335) Co-authored-by: Pelle <pellekoster@octagonal.nl> * Adding capability of importing and exporting networks in GMNS format (#330) * Pedro/fixes numpy changes (#342) * Fixes nan to num * Chipping along transit structure * Fixes tests * Uses new version of Numpy for Windows 3.10 only * address issues from review * unneeded Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> Co-authored-by: Jamie Cook <jimi.cook@gmail.com> * Updates actions (#341) * Updates actions * Chipping along transit structure Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> Co-authored-by: Jamie Cook <jamie.cook@veitchlister.com.au> * Fixes issue #344 (#345) * Fixes issue #344 Co-authored-by: François P <35044397+djfrancesco@users.noreply.github.com> * Bumps up version * Bumps up version (#346) Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> * bug fix (#347) * Updates actions (#349) Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> * Update Develop * stops using Kumi endpoint Co-authored-by: Danilo Ebbinghaus <91709021+daniloebbinghaus@users.noreply.github.com> Co-authored-by: Arthur Evrard <83211842+Art-Ev@users.noreply.github.com> Co-authored-by: Jan Zill <jan.zill@veitchlister.com.au> Co-authored-by: Jan <jan.christoph.zill@gmail.com> Co-authored-by: Jamie Cook <jamie.cook@veitchlister.com.au> Co-authored-by: PelleK <elfjes@users.noreply.github.com> Co-authored-by: Pelle Koster <pelle.koster@nginfra.nl> Co-authored-by: pveigadecamargo <pveigadecamargo@anl.gov> Co-authored-by: Jamie Cook <jimi.cook@gmail.com> Co-authored-by: Pelle <pellekoster@octagonal.nl>
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.
Currently, only a single project can be openened by Aequilibrae, stored in the
AEQUILIBRAE_PROJECT_PATHenvironment variable. This PR implements support for multiple projects to be loaded at the same time. It does this by binding the classes that deal with project data to a specific project (ie: providing an (optional) project constructor argument). In order to maintain backwards compatibility, it also introduces the concept of an active project (seecontext.py). A project activates itself upon opening/loading, but can also be explicitely activated or deactivated. When instantiating a calculation class (such asTrafficAssignment) it is possible to give a project as an argument (TrafficAssignment(project=project)) or not (TrafficAssignment()). In the latter case the currently active project will be usedHaving multiple projects open at the same time also means that the table classes such as
Linkscan no longer assume there is a single list ofitems.itemshas been moved to the instance instead of a being class variable. It loses its singleton behaviour, so care must be taken to only instantiate these classes once per project. These instances are now bound to the project or the project's network (as far as they weren't bound already) and should not be created by other classesFurthermore this PR consists of the following changes:
Project.connect()instead of callingdatabase_connectionmanuallyFileExistsErrortoFileNotFoundErrorbackwards compatibility
This PR is mostly backwards compatible. The active project concept ensures that most classes can still be instantiated without a specific project. There are a few considerations however:
AEQUILIBRAE_PROJECT_PATHenvironment variable was only set inside the application when opening a new or existing project and never read from an outside setter. (eg. a user exportingAEQUILIBRAE_PROJECT_PATHprior to starting python / importing aequilibrae. In this PRAEQUILIBRAE_PROJECT_PATHis no longer used, so if there is any use case for setting AEQUILIBRAE_PROJECT_PATH from outside Aequilibrae, this no longer works (although this functionality could be added again)FileExistsErrortoFileNotFoundErroris a breaking changeWhat do you guys think? I have not created an Issue for this (can still do that if desired), but just went ahead and created this PR. If you think it needs a different approach, that's also possible, but I thought this is a/the way that integrates best in the current design.