Provide more flexibility for defining mandatory schedule specifications. - #275

Merged
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex
Dec 17, 2019
Merged

Provide more flexibility for defining mandatory schedule specifications.#275
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex

Conversation

@danielsclint

@danielsclintdanielsclint commented Dec 5, 2019

Copy link
Copy Markdown

#273. This change require defining tour scheduling specs specifically in the mandatory_tour_scheduling.yaml. The code still hard-codes the 'univ' value, and additional flexibility should be added in the future. This is step one of probably a couple of steps to unwind the hard-coded values in this section of the code.

SPEC:
work: tour_scheduling_work.csv
school: tour_scheduling_school.csv
univ: tour_scheduling_university.csv

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    The example scripts have been updated so the examples and tests complete. It's unclear how the documentation should be updated for this change. Few of the internal workings of model settings are well-documented, and it seems like a large discussion (or work effort) is needed to standardize this across the models.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes, this change improves the flexibility of the ActivitySim framework allowing it be implemented in more locations.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change will have no material change on the model run time. This code change replaces hard-coded values with a set of user-defined parameters.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done. The code change removes a hard-coded value in the Python code with a set of user-defined variables. If the user specifies the same values as the previously hard-coded values, they should get the same results. The unit tests seem to confirm this assertion.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    The test configuration files were modified to use the newest features.

  9. Any other comments or suggestions for improving the developer experience?
    The documentation is still being built out, so it is difficult to understand where and how to add new documentation into the existing framework. Some documentation should be consistent across all models (as is the case with this change), so a standard would need to be set by the larger team or asserted by me.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 86.123% when pulling c9f890e on danielsclint:ft_schedule_flex into 7b57c94 on ActivitySim:master.

@danielsclint

Copy link
Copy Markdown
Author

@bstabler, I updated the original PR comment with the some additional documentation and answered the "Review Criteria" questions. This PR works, but as I note in the comment, it probably should be followed-up with a few more code enhancements to make the assertion of the 'univ' trips more flexible.

@bstabler
bstabler changed the base branch from master to developDecember 17, 2019 22:57
@bstabler

Copy link
Copy Markdown
Contributor

This looks good and we accept the PR. I changed the target to the develop branch so we can merge features here before merging (releasing) to master.

@bstabler
bstabler merged commit 7db24a6 into ActivitySim:developDec 17, 2019
@danielsclint
danielsclint deleted the ft_schedule_flex branch December 20, 2019 19:41
@bstablerbstabler mentioned this pull request Dec 23, 2019
Sign up for freeto 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.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Provide more flexibility for defining mandatory schedule specifications. - #275

Merged
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex
Dec 17, 2019
Merged

Provide more flexibility for defining mandatory schedule specifications.#275
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex

Conversation

@danielsclint

@danielsclintdanielsclint commented Dec 5, 2019

Copy link
Copy Markdown

#273. This change require defining tour scheduling specs specifically in the mandatory_tour_scheduling.yaml. The code still hard-codes the 'univ' value, and additional flexibility should be added in the future. This is step one of probably a couple of steps to unwind the hard-coded values in this section of the code.

SPEC:
work: tour_scheduling_work.csv
school: tour_scheduling_school.csv
univ: tour_scheduling_university.csv

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    The example scripts have been updated so the examples and tests complete. It's unclear how the documentation should be updated for this change. Few of the internal workings of model settings are well-documented, and it seems like a large discussion (or work effort) is needed to standardize this across the models.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes, this change improves the flexibility of the ActivitySim framework allowing it be implemented in more locations.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change will have no material change on the model run time. This code change replaces hard-coded values with a set of user-defined parameters.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done. The code change removes a hard-coded value in the Python code with a set of user-defined variables. If the user specifies the same values as the previously hard-coded values, they should get the same results. The unit tests seem to confirm this assertion.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    The test configuration files were modified to use the newest features.

  9. Any other comments or suggestions for improving the developer experience?
    The documentation is still being built out, so it is difficult to understand where and how to add new documentation into the existing framework. Some documentation should be consistent across all models (as is the case with this change), so a standard would need to be set by the larger team or asserted by me.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 86.123% when pulling c9f890e on danielsclint:ft_schedule_flex into 7b57c94 on ActivitySim:master.

@danielsclint

Copy link
Copy Markdown
Author

@bstabler, I updated the original PR comment with the some additional documentation and answered the "Review Criteria" questions. This PR works, but as I note in the comment, it probably should be followed-up with a few more code enhancements to make the assertion of the 'univ' trips more flexible.

@bstabler
bstabler changed the base branch from master to developDecember 17, 2019 22:57
@bstabler

Copy link
Copy Markdown
Contributor

This looks good and we accept the PR. I changed the target to the develop branch so we can merge features here before merging (releasing) to master.

@bstabler
bstabler merged commit 7db24a6 into ActivitySim:developDec 17, 2019
@danielsclint
danielsclint deleted the ft_schedule_flex branch December 20, 2019 19:41
@bstablerbstabler mentioned this pull request Dec 23, 2019
Sign up for freeto 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.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Provide more flexibility for defining mandatory schedule specifications. - #275

Merged
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex
Dec 17, 2019
Merged

Provide more flexibility for defining mandatory schedule specifications.#275
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex

Conversation

@danielsclint

@danielsclintdanielsclint commented Dec 5, 2019

Copy link
Copy Markdown

#273. This change require defining tour scheduling specs specifically in the mandatory_tour_scheduling.yaml. The code still hard-codes the 'univ' value, and additional flexibility should be added in the future. This is step one of probably a couple of steps to unwind the hard-coded values in this section of the code.

SPEC:
work: tour_scheduling_work.csv
school: tour_scheduling_school.csv
univ: tour_scheduling_university.csv

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    The example scripts have been updated so the examples and tests complete. It's unclear how the documentation should be updated for this change. Few of the internal workings of model settings are well-documented, and it seems like a large discussion (or work effort) is needed to standardize this across the models.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes, this change improves the flexibility of the ActivitySim framework allowing it be implemented in more locations.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change will have no material change on the model run time. This code change replaces hard-coded values with a set of user-defined parameters.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done. The code change removes a hard-coded value in the Python code with a set of user-defined variables. If the user specifies the same values as the previously hard-coded values, they should get the same results. The unit tests seem to confirm this assertion.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    The test configuration files were modified to use the newest features.

  9. Any other comments or suggestions for improving the developer experience?
    The documentation is still being built out, so it is difficult to understand where and how to add new documentation into the existing framework. Some documentation should be consistent across all models (as is the case with this change), so a standard would need to be set by the larger team or asserted by me.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 86.123% when pulling c9f890e on danielsclint:ft_schedule_flex into 7b57c94 on ActivitySim:master.

@danielsclint

Copy link
Copy Markdown
Author

@bstabler, I updated the original PR comment with the some additional documentation and answered the "Review Criteria" questions. This PR works, but as I note in the comment, it probably should be followed-up with a few more code enhancements to make the assertion of the 'univ' trips more flexible.

@bstabler
bstabler changed the base branch from master to developDecember 17, 2019 22:57
@bstabler

Copy link
Copy Markdown
Contributor

This looks good and we accept the PR. I changed the target to the develop branch so we can merge features here before merging (releasing) to master.

@bstabler
bstabler merged commit 7db24a6 into ActivitySim:developDec 17, 2019
@danielsclint
danielsclint deleted the ft_schedule_flex branch December 20, 2019 19:41
@bstablerbstabler mentioned this pull request Dec 23, 2019
Sign up for freeto 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.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Provide more flexibility for defining mandatory schedule specifications. - #275

Merged
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex
Dec 17, 2019
Merged

Provide more flexibility for defining mandatory schedule specifications.#275
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex

Conversation

@danielsclint

@danielsclintdanielsclint commented Dec 5, 2019

Copy link
Copy Markdown

#273. This change require defining tour scheduling specs specifically in the mandatory_tour_scheduling.yaml. The code still hard-codes the 'univ' value, and additional flexibility should be added in the future. This is step one of probably a couple of steps to unwind the hard-coded values in this section of the code.

SPEC:
work: tour_scheduling_work.csv
school: tour_scheduling_school.csv
univ: tour_scheduling_university.csv

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    The example scripts have been updated so the examples and tests complete. It's unclear how the documentation should be updated for this change. Few of the internal workings of model settings are well-documented, and it seems like a large discussion (or work effort) is needed to standardize this across the models.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes, this change improves the flexibility of the ActivitySim framework allowing it be implemented in more locations.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change will have no material change on the model run time. This code change replaces hard-coded values with a set of user-defined parameters.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done. The code change removes a hard-coded value in the Python code with a set of user-defined variables. If the user specifies the same values as the previously hard-coded values, they should get the same results. The unit tests seem to confirm this assertion.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    The test configuration files were modified to use the newest features.

  9. Any other comments or suggestions for improving the developer experience?
    The documentation is still being built out, so it is difficult to understand where and how to add new documentation into the existing framework. Some documentation should be consistent across all models (as is the case with this change), so a standard would need to be set by the larger team or asserted by me.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 86.123% when pulling c9f890e on danielsclint:ft_schedule_flex into 7b57c94 on ActivitySim:master.

@danielsclint

Copy link
Copy Markdown
Author

@bstabler, I updated the original PR comment with the some additional documentation and answered the "Review Criteria" questions. This PR works, but as I note in the comment, it probably should be followed-up with a few more code enhancements to make the assertion of the 'univ' trips more flexible.

@bstabler
bstabler changed the base branch from master to developDecember 17, 2019 22:57
@bstabler

Copy link
Copy Markdown
Contributor

This looks good and we accept the PR. I changed the target to the develop branch so we can merge features here before merging (releasing) to master.

@bstabler
bstabler merged commit 7db24a6 into ActivitySim:developDec 17, 2019
@danielsclint
danielsclint deleted the ft_schedule_flex branch December 20, 2019 19:41
@bstablerbstabler mentioned this pull request Dec 23, 2019
Sign up for freeto 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.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Provide more flexibility for defining mandatory schedule specifications. - #275

Merged
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex
Dec 17, 2019
Merged

Provide more flexibility for defining mandatory schedule specifications.#275
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex

Conversation

@danielsclint

@danielsclintdanielsclint commented Dec 5, 2019

Copy link
Copy Markdown

#273. This change require defining tour scheduling specs specifically in the mandatory_tour_scheduling.yaml. The code still hard-codes the 'univ' value, and additional flexibility should be added in the future. This is step one of probably a couple of steps to unwind the hard-coded values in this section of the code.

SPEC:
work: tour_scheduling_work.csv
school: tour_scheduling_school.csv
univ: tour_scheduling_university.csv

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    The example scripts have been updated so the examples and tests complete. It's unclear how the documentation should be updated for this change. Few of the internal workings of model settings are well-documented, and it seems like a large discussion (or work effort) is needed to standardize this across the models.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes, this change improves the flexibility of the ActivitySim framework allowing it be implemented in more locations.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change will have no material change on the model run time. This code change replaces hard-coded values with a set of user-defined parameters.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done. The code change removes a hard-coded value in the Python code with a set of user-defined variables. If the user specifies the same values as the previously hard-coded values, they should get the same results. The unit tests seem to confirm this assertion.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    The test configuration files were modified to use the newest features.

  9. Any other comments or suggestions for improving the developer experience?
    The documentation is still being built out, so it is difficult to understand where and how to add new documentation into the existing framework. Some documentation should be consistent across all models (as is the case with this change), so a standard would need to be set by the larger team or asserted by me.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 86.123% when pulling c9f890e on danielsclint:ft_schedule_flex into 7b57c94 on ActivitySim:master.

@danielsclint

Copy link
Copy Markdown
Author

@bstabler, I updated the original PR comment with the some additional documentation and answered the "Review Criteria" questions. This PR works, but as I note in the comment, it probably should be followed-up with a few more code enhancements to make the assertion of the 'univ' trips more flexible.

@bstabler
bstabler changed the base branch from master to developDecember 17, 2019 22:57
@bstabler

Copy link
Copy Markdown
Contributor

This looks good and we accept the PR. I changed the target to the develop branch so we can merge features here before merging (releasing) to master.

@bstabler
bstabler merged commit 7db24a6 into ActivitySim:developDec 17, 2019
@danielsclint
danielsclint deleted the ft_schedule_flex branch December 20, 2019 19:41
@bstablerbstabler mentioned this pull request Dec 23, 2019
Sign up for freeto 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.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Provide more flexibility for defining mandatory schedule specifications. - #275

Merged
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex
Dec 17, 2019
Merged

Provide more flexibility for defining mandatory schedule specifications.#275
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex

Conversation

@danielsclint

@danielsclintdanielsclint commented Dec 5, 2019

Copy link
Copy Markdown

#273. This change require defining tour scheduling specs specifically in the mandatory_tour_scheduling.yaml. The code still hard-codes the 'univ' value, and additional flexibility should be added in the future. This is step one of probably a couple of steps to unwind the hard-coded values in this section of the code.

SPEC:
work: tour_scheduling_work.csv
school: tour_scheduling_school.csv
univ: tour_scheduling_university.csv

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    The example scripts have been updated so the examples and tests complete. It's unclear how the documentation should be updated for this change. Few of the internal workings of model settings are well-documented, and it seems like a large discussion (or work effort) is needed to standardize this across the models.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes, this change improves the flexibility of the ActivitySim framework allowing it be implemented in more locations.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change will have no material change on the model run time. This code change replaces hard-coded values with a set of user-defined parameters.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done. The code change removes a hard-coded value in the Python code with a set of user-defined variables. If the user specifies the same values as the previously hard-coded values, they should get the same results. The unit tests seem to confirm this assertion.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    The test configuration files were modified to use the newest features.

  9. Any other comments or suggestions for improving the developer experience?
    The documentation is still being built out, so it is difficult to understand where and how to add new documentation into the existing framework. Some documentation should be consistent across all models (as is the case with this change), so a standard would need to be set by the larger team or asserted by me.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 86.123% when pulling c9f890e on danielsclint:ft_schedule_flex into 7b57c94 on ActivitySim:master.

@danielsclint

Copy link
Copy Markdown
Author

@bstabler, I updated the original PR comment with the some additional documentation and answered the "Review Criteria" questions. This PR works, but as I note in the comment, it probably should be followed-up with a few more code enhancements to make the assertion of the 'univ' trips more flexible.

@bstabler
bstabler changed the base branch from master to developDecember 17, 2019 22:57
@bstabler

Copy link
Copy Markdown
Contributor

This looks good and we accept the PR. I changed the target to the develop branch so we can merge features here before merging (releasing) to master.

@bstabler
bstabler merged commit 7db24a6 into ActivitySim:developDec 17, 2019
@danielsclint
danielsclint deleted the ft_schedule_flex branch December 20, 2019 19:41
@bstablerbstabler mentioned this pull request Dec 23, 2019
Sign up for freeto 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.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Provide more flexibility for defining mandatory schedule specifications. - #275

Merged
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex
Dec 17, 2019
Merged

Provide more flexibility for defining mandatory schedule specifications.#275
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex

Conversation

@danielsclint

@danielsclintdanielsclint commented Dec 5, 2019

Copy link
Copy Markdown

#273. This change require defining tour scheduling specs specifically in the mandatory_tour_scheduling.yaml. The code still hard-codes the 'univ' value, and additional flexibility should be added in the future. This is step one of probably a couple of steps to unwind the hard-coded values in this section of the code.

SPEC:
work: tour_scheduling_work.csv
school: tour_scheduling_school.csv
univ: tour_scheduling_university.csv

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    The example scripts have been updated so the examples and tests complete. It's unclear how the documentation should be updated for this change. Few of the internal workings of model settings are well-documented, and it seems like a large discussion (or work effort) is needed to standardize this across the models.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes, this change improves the flexibility of the ActivitySim framework allowing it be implemented in more locations.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change will have no material change on the model run time. This code change replaces hard-coded values with a set of user-defined parameters.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done. The code change removes a hard-coded value in the Python code with a set of user-defined variables. If the user specifies the same values as the previously hard-coded values, they should get the same results. The unit tests seem to confirm this assertion.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    The test configuration files were modified to use the newest features.

  9. Any other comments or suggestions for improving the developer experience?
    The documentation is still being built out, so it is difficult to understand where and how to add new documentation into the existing framework. Some documentation should be consistent across all models (as is the case with this change), so a standard would need to be set by the larger team or asserted by me.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 86.123% when pulling c9f890e on danielsclint:ft_schedule_flex into 7b57c94 on ActivitySim:master.

@danielsclint

Copy link
Copy Markdown
Author

@bstabler, I updated the original PR comment with the some additional documentation and answered the "Review Criteria" questions. This PR works, but as I note in the comment, it probably should be followed-up with a few more code enhancements to make the assertion of the 'univ' trips more flexible.

@bstabler
bstabler changed the base branch from master to developDecember 17, 2019 22:57
@bstabler

Copy link
Copy Markdown
Contributor

This looks good and we accept the PR. I changed the target to the develop branch so we can merge features here before merging (releasing) to master.

@bstabler
bstabler merged commit 7db24a6 into ActivitySim:developDec 17, 2019
@danielsclint
danielsclint deleted the ft_schedule_flex branch December 20, 2019 19:41
@bstablerbstabler mentioned this pull request Dec 23, 2019
Sign up for freeto 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.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Provide more flexibility for defining mandatory schedule specifications. - #275

Merged
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex
Dec 17, 2019
Merged

Provide more flexibility for defining mandatory schedule specifications.#275
bstabler merged 7 commits into
ActivitySim:developfrom
danielsclint:ft_schedule_flex

Conversation

@danielsclint

@danielsclintdanielsclint commented Dec 5, 2019

Copy link
Copy Markdown

#273. This change require defining tour scheduling specs specifically in the mandatory_tour_scheduling.yaml. The code still hard-codes the 'univ' value, and additional flexibility should be added in the future. This is step one of probably a couple of steps to unwind the hard-coded values in this section of the code.

SPEC:
work: tour_scheduling_work.csv
school: tour_scheduling_school.csv
univ: tour_scheduling_university.csv

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    The example scripts have been updated so the examples and tests complete. It's unclear how the documentation should be updated for this change. Few of the internal workings of model settings are well-documented, and it seems like a large discussion (or work effort) is needed to standardize this across the models.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes, this change improves the flexibility of the ActivitySim framework allowing it be implemented in more locations.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change will have no material change on the model run time. This code change replaces hard-coded values with a set of user-defined parameters.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done. The code change removes a hard-coded value in the Python code with a set of user-defined variables. If the user specifies the same values as the previously hard-coded values, they should get the same results. The unit tests seem to confirm this assertion.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    The test configuration files were modified to use the newest features.

  9. Any other comments or suggestions for improving the developer experience?
    The documentation is still being built out, so it is difficult to understand where and how to add new documentation into the existing framework. Some documentation should be consistent across all models (as is the case with this change), so a standard would need to be set by the larger team or asserted by me.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.02%) to 86.123% when pulling c9f890e on danielsclint:ft_schedule_flex into 7b57c94 on ActivitySim:master.

@danielsclint

Copy link
Copy Markdown
Author

@bstabler, I updated the original PR comment with the some additional documentation and answered the "Review Criteria" questions. This PR works, but as I note in the comment, it probably should be followed-up with a few more code enhancements to make the assertion of the 'univ' trips more flexible.

@bstabler
bstabler changed the base branch from master to developDecember 17, 2019 22:57
@bstabler

Copy link
Copy Markdown
Contributor

This looks good and we accept the PR. I changed the target to the develop branch so we can merge features here before merging (releasing) to master.

@bstabler
bstabler merged commit 7db24a6 into ActivitySim:developDec 17, 2019
@danielsclint
danielsclint deleted the ft_schedule_flex branch December 20, 2019 19:41
@bstablerbstabler mentioned this pull request Dec 23, 2019
Sign up for freeto 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.

3 participants

@danielsclint@coveralls@bstabler