-
Notifications
You must be signed in to change notification settings - Fork 196
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
Schedule:File relative paths #4879
Merged
Merged
Changes from 35 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
6a4767b
Add new ft test for schedule file relative path.
joseph-robertson e13f101
Try filePath as relative path for FT switch.
joseph-robertson 1b8cfff
Formatting.
joseph-robertson c4b78b8
Updates to new test.
joseph-robertson 2637831
Missing include and minor cleanup.
joseph-robertson 9fee86f
Add more ft tests for different schedule file ctors.
joseph-robertson d60f631
Missing includes.
joseph-robertson 91b7f64
Use the correct external file ctor.
joseph-robertson 014991a
Fix tests.
joseph-robertson 76c8024
Only error check on path existence if copyFile is true or path is abs…
joseph-robertson cc84c90
Add new ft test for schedule file relative path.
joseph-robertson fbcf19b
Try filePath as relative path for FT switch.
joseph-robertson 40a5e3a
Formatting.
joseph-robertson 838edd0
Updates to new test.
joseph-robertson f0c7b5b
Missing include and minor cleanup.
joseph-robertson 67a9567
Add more ft tests for different schedule file ctors.
joseph-robertson d429cf9
Missing includes.
joseph-robertson 500574f
Use the correct external file ctor.
joseph-robertson e935dae
Fix tests.
joseph-robertson bdddd1f
Only error check on path existence if copyFile is true or path is abs…
joseph-robertson 528ac24
Merge branch 'schedule-file-rel-path' of github.com:NREL/OpenStudio i…
joseph-robertson 3001627
Merge branch 'develop' into schedule-file-rel-path
joseph-robertson 4cc1a59
Updates for externalfile and new ft tests.
joseph-robertson 2a30424
Update externalfile idd to have bool translatefilename field.
joseph-robertson 9c0c625
Update ft to use translatefilename.
joseph-robertson 4e3bc6c
Update externalfile files for new field.
joseph-robertson e3aa21d
Update schedulefile ctor for new field.
joseph-robertson 48c7986
Get the field name correct.
joseph-robertson 4c5591c
Typo.
joseph-robertson 045abc0
Move translatefilename over to schedulefile instead.
joseph-robertson c4d2572
Update schedulefile ctor.
joseph-robertson 6451066
Add methods for defaulted and reset.
joseph-robertson 0116c68
Updates to model and ft tests.
joseph-robertson 247cdef
Add choice to idd.
joseph-robertson 7345dea
Update new ft tests.
joseph-robertson 30e34f1
Merge branch 'develop' into schedule-file-rel-path
jmarrec d64e7fb
Rename IDD Field, move path logic into model `openstudio::path Schedu…
jmarrec fb604f5
Make tests platform and current directory agnostics
jmarrec c008154
Merge pull request #5010 from NREL/schedule-file-rel-path-2
jmarrec 28a3f30
Merge branch 'develop' into schedule-file-rel-path
DavidGoldwasser File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,16 +41,21 @@ namespace energyplus { | |
} | ||
} | ||
|
||
path filePath = modelObject.externalFile().filePath(); | ||
if (!exists(filePath)) { | ||
LOG(Warn, "Cannot find file \"" << filePath << "\""); | ||
path fileName; | ||
if (modelObject.translateFileName()) { | ||
fileName = toPath(modelObject.externalFile().fileName()); | ||
} else { | ||
// make the path correct for this system | ||
filePath = system_complete(filePath); | ||
fileName = modelObject.externalFile().filePath(); | ||
if (!exists(fileName)) { | ||
LOG(Warn, "Cannot find file \"" << fileName << "\""); | ||
} else { | ||
// make the path correct for this system | ||
fileName = system_complete(fileName); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider putting this logic in ScheduleFile directly, that'd be much easier to debug if something goes awry. And much easier to add a model test for it too. openstudio::path ScheduleFile::translatedFilePath() const {
if (translatedFileName()) {
return toPath(modelObject.externalFile().fileName());
}
openstudio::path filePath = modelObject.externalFile().filePath();
if (!exists(filePath)) {
LOG(Warn, "Cannot find file \"" << filePath << "\"");
} else {
// make the path correct for this system
filePath = system_complete(filePath);
}
return filePath;
} (If externalFile.filePath doesn't exist, maybe it should throw?) |
||
} | ||
|
||
// DLM: this path is going to be in the temp dir, might want to fix it up when saving model temp dir | ||
idfObject.setString(openstudio::Schedule_FileFields::FileName, toString(filePath)); | ||
idfObject.setString(openstudio::Schedule_FileFields::FileName, toString(fileName)); | ||
|
||
idfObject.setInt(openstudio::Schedule_FileFields::ColumnNumber, modelObject.columnNumber()); | ||
idfObject.setInt(openstudio::Schedule_FileFields::RowstoSkipatTop, modelObject.rowstoSkipatTop()); | ||
|
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
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the name is clear. Should have a
\note
probably as well.Not a huge fan of having a default rather than hard-coded required param. I know this saves on needing to do VT, and the above has a default, so I guess it's up to you to make it required or not.
Maybe one of these two?