Closed Myoldmopar closed 8 months ago
Some things I'd like to wrap up before merge:
None
for design-day-only rather than the magic string "no-weather-file"?@JasonGlazer OK, this is ready for review. The best way will be to just show off the tool on a meeting so I'll reach out to you separately to set that up.
This should be equivalent or very very close to the wx version, but with a much improved packaging/deployment experience. I have pip installed it on all three platforms and it behaves perfectly and look good on all of them.
Noticed there is something up with the weather file path. It will try to find the weather file in the current working directory, at least sometimes. Need to scan the code to protect against that. I will make a full pass over the code looking for other things to protect while I'm in there.
OK, looks like the weather file issue was just due to something that is already fixed in the code. At some point we were only keeping the weather file name in the cache, but it's always a full path or the "
Audit file issue appears to be inside E+ itself. I could reproduce the issue easily, just run the same file twice in a row. I then went into the E+ code and tried manually closing a chunk of files inside CloseOutputFiles, and built a new package. When I run that in EPLaunch, it can run as many times as it wants successfully. The file descriptors are simply being held on Windows for some reason. I'm going to figure out the right spot inside E+ to close the files and build a dummy release tag and see how it goes.
To be honest I would not have expected that EnergyPlus not releasing a file was to blame. It is used in so many contexts, I am surprised it hadn't been found earlier.
Taskbar icon issue is now fixed. As far as I am concerned, this can merge in. But honestly, now that these little annoying paper cuts are cleaned up, I would be happy to take a couple hours reworking anything on the GUI that needs cleaned up. @JasonGlazer we could chat on Monday to discuss any final polishing you'd like done. Some things that come to mind:
These are all small efforts, and now that the really annoying things are behind me, I'm open to dropping a few hours to wrap things like this up to maximize the usefulness.
Went ahead and added a couple tweaks this morning. I created a new dialog that displays the available keyboard shortcuts. I considered modifying the group menu and moving it down to the group section, but I felt like the UI had enough buttons, so I aborted. Once I get to test this out across platforms, I'm happy to call this done.
I noticed that the dialogs didn't have the same E+ icon, so fixed that up. It looks very nice now :star_struck:
Hey @JasonGlazer, do you have objections to merging this in? I'm looking over all the Python apps and am anxious to get this merged and properly releasing from the main
branch so I can proceed cleanly toward the pip install energyplus
world. If you are good with it, I'll bump the version one more time before we merge/release.
@Myoldmopar I haven't looked this over in a long time and never did a real review. I did some random UI testing only. If you are looking for a more careful review, I will work on it after I finish up the bug fix I'm working on.
Honestly, I think we should just merge it in. I am happy to give you more walkthrough and such, but I feel confident that we have reached a point that we need to get it "released" so that we can do these same fixups across our other tools. This is not perfect, I'm sure, but it's pretty darn good. Let's merge it.
Great step forward!
FYI @JasonGlazer
Enormous set of changes, but at this point I have it cleanly building and posting to PyPi. I have coverage back up to 100%, and Flake8 all happy. I'd like to show it off and discuss it with you as the functionality is almost all in place, but there are certainly some little tweaks to make before merge.