Wrap PythonPlugin:SearchPaths by joseph-robertson · Pull Request #5312 · NatLabRockies/OpenStudio

joseph-robertson · 2024-12-02T20:55:30Z

Pull request overview

Fixes E+ 24.2.1: Wrap PythonPlugin:SearchPaths #5269

Pull Request Author

Model API Changes / Additions
Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
Model API methods are tested (in src/model/test)
EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
If a new object or method, added a test in NREL/OpenStudio-resources:
- Wrap PythonPlugin:SearchPaths OpenStudio-resources#214
If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
All new and existing tests passes
If methods have been deprecated, update rest of code to use the new methods

Labels:

If change to an IDD file, add the label IDDChange
If breaking existing API, add the label APIChange
If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
Labeling is ok
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
If feature, test running new feature, try creative ways to break it
CI status: all green or justified

jmarrec · 2024-12-12T11:01:53Z

Openstudio-resources test wanted I guess.

Create a model/simualtiontests/python_plugin_search_paths.py

Probably make a dumb py file next to it, at model/simulationtests/python_plugin_search_paths_script.py that has a dumb function or just a print statement in it.

In model/simualtiontests/python_plugin_search_paths.py make a python plugin script that would add the Path(__file__).parent to the search paths and would do import python_plugin_search_paths_script. It should fail when the PythonPluginSearchPaths isn't used, and work with it.

jmarrec · 2024-12-12T11:03:37Z

cppcheck failing

[src/model/PythonPluginSearchPaths.hpp:52]:(style),[missingOverride],The destructor '~PythonPluginSearchPaths' overrides a destructor in a base class but is not marked with a 'override' specifier.
[src/model/PythonPluginSearchPaths_Impl.hpp:54]:(style),[missingOverride],The destructor '~PythonPluginSearchPaths_Impl' overrides a destructor in a base class but is not marked with a 'override' specifier.

jmarrec

I think it should take path. Or at least it should ALLOW a path.

If taking a path, you want to store it as a path.generic_string() (forward slashes always) as this is eventually what E+ will do when addToPythonPath is called.

jmarrec · 2024-12-19T10:23:33Z

src/model/PythonPluginSearchPaths.hpp

+    bool addSearchPath(const std::string& searchPath);
+
+    bool setSearchPaths(const std::vector<std::string>& searchPaths);


Hum, std::string. Shouldn't this be openstudio::path?

I'm kinda torn on making it accept ONLY a Path, or allowing both. if allowing both, the std::string version should forward to the path one, and not the opposite

jmarrec · 2024-12-19T11:50:23Z

src/model/PythonPluginSearchPaths.hpp

+  /** PythonPluginSearchPaths is a ParentObject that wraps the OpenStudio IDD object 'OS:PythonPlugin:SearchPaths'. */
+  class MODEL_API PythonPluginSearchPaths : public ParentObject


Wait, why would it be a ParentObject?!

jmarrec · 2024-12-19T11:54:30Z

src/model/PythonPluginSearchPaths.cpp

+      std::vector<std::string> existingSearchPaths = this->searchPaths();
+      if (std::find_if(existingSearchPaths.begin(), existingSearchPaths.end(),
+                       [&searchPath](const std::string& s) { return openstudio::istringEqual(s, searchPath); })
+          != existingSearchPaths.end()) {
+        LOG(Info, "Not adding search path '" << searchPath << "' to PythonPlugin:SearchPaths since it is already present");
+        // Return true anyways, it's a success
+        return true;
+      }


Unix is case sensitive!

…ddSearchPath/setSearchPaths for convenience

…o remove the std::string one

jmarrec · 2024-12-19T14:55:37Z

src/model/PythonPluginSearchPaths.hpp

+  }  // namespace detail
+
+  /** PythonPluginSearchPaths is a ModelObject that wraps the OpenStudio IDD object 'OS:PythonPlugin:SearchPaths'. */
+  class MODEL_API PythonPluginSearchPaths : public ModelObject


Made it a ModelObject, since it does not parent anything

jmarrec · 2024-12-19T14:55:43Z

src/model/PythonPluginSearchPaths.hpp

+
+    bool addepinEnvironmentVariabletoSearchPath() const;
+
+    std::vector<openstudio::path> searchPaths() const;


jmarrec · 2024-12-19T14:57:34Z

src/model/PythonPluginSearchPaths.hpp

+    bool addSearchPath(const openstudio::path& searchPath);
+    bool setSearchPaths(const std::vector<openstudio::path>& searchPaths);
+
+    // Convenience, forwards to the openstudio::path equivalent
+    bool setSearchPaths(const std::vector<std::string>& searchPaths);


use paths. I kept the std::vectorstd::string for convenience.

I had kept the bool addSearchPath(const std::string& searchPath); one to begin with, but the call to addSearchPath("string litteral") was ambiguous since const char * converts to both std::string and fs::path, so I removed it (instead of adding a third addSearchPath(const char*) overload, and having to ignore it in SWIG...)

You can still do this in ruby:

[1] OS-build-release(main)> m = Model.new => #<OpenStudio::Model::Model:0x00007b0500a14d90 @__swigtype__="_p_openstudio__model__Model"> [2] OS-build-release(main)> p = m.getPythonPluginSearchPaths => #<OpenStudio::Model::PythonPluginSearchPaths:0x00007b05005079b0 @__swigtype__="_p_openstudio__model__PythonPluginSearchPaths"> [3] OS-build-release(main)> p.addSearchPath("lol") => true [4] OS-build-release(main)> puts p OS:PythonPlugin:SearchPaths, {85828d9f-b7df-44f7-89c4-cbd18c92badc}, !- Handle Yes, !- Add Current Working Directory to Search Path Yes, !- Add Input File Directory to Search Path Yes, !- Add epin Environment Variable to Search Path lol; !- Search Path 1

Python:

In [1]: m = openstudio.model.Model() In [2]: p = m.getPythonPluginSearchPaths() In [3]: p.addSearchPath(Path('.').absolute()) Out[3]: True In [4]: p.addSearchPath('the path') Out[4]: True In [5]: print(p) OS:PythonPlugin:SearchPaths, {c965de9e-6e78-4a6a-aa53-5041e1ac5809}, !- Handle Yes, !- Add Current Working Directory to Search Path Yes, !- Add Input File Directory to Search Path Yes, !- Add epin Environment Variable to Search Path /home/julien/Software/Others/EnergyPlus-build-release, !- Search Path 1 the path; !- Search Path 2

jmarrec · 2024-12-19T15:01:58Z

I have no idea why the EpwFile / SQFile tests are segfaulting on Ubuntu 24.04. I have the same machine locally and they run fine.

ci-commercialbuildings · 2024-12-19T15:55:15Z

CI Results for 54a43e6:

Ubuntu 20.04:
Ubuntu 22.04:
Darwin:
Windows:
- OpenStudio-3.9.1-alpha+54a43e6de3-Windows.exe
- OpenStudio-3.9.1-alpha+54a43e6de3-Windows.tar.gz
Code Coverage:

joseph-robertson added Enhancement Request component - Model Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Dec 2, 2024

joseph-robertson self-assigned this Dec 2, 2024

joseph-robertson marked this pull request as ready for review December 6, 2024 22:41

joseph-robertson mentioned this pull request Dec 12, 2024

Wrap PythonPlugin:SearchPaths NatLabRockies/OpenStudio-resources#214

Merged

26 tasks

joseph-robertson requested a review from jmarrec December 13, 2024 21:40

jmarrec requested changes Dec 19, 2024

View reviewed changes

jmarrec force-pushed the python-plugin-search-paths branch from e95cb3a to 4613368 Compare December 19, 2024 12:00

joseph-robertson and others added 17 commits December 19, 2024 15:39

Add pythonplugin:searchpaths object to idd.

2eb43d1

Generate pythonpluginsearchpaths class with script.

541c80e

Fill out extensible methods.

47c4068

Formatting.

d136297

Update cmakelists, model files, partial classes, etc.

fe3461a

Update ft hpp and cpp.

29a3f01

Fix model test ctor and fill out extensible field tests.

e5e4b07

Fix and update the ft test.

2746023

Clean up ft and add extensible groups.

51ceb0f

Move searchpaths ft before instance ft.

12e5f32

Update model and ft tests.

1ac5bb6

Add override to destructor.

991022b

Update ctor to set required fields.

d05a5aa

Update ft test.

deeb88f

Make it be a ModelObject, it's got no business being a ParentObject

a24430a

Make the implementation use openstudio::path, keeping a std::string a…

7f1cac2

…ddSearchPath/setSearchPaths for convenience

Overloaded fs::path and std::string addSearchPath call is ambigous, s…

187352f

…o remove the std::string one

jmarrec force-pushed the python-plugin-search-paths branch from 54a43e6 to 187352f Compare December 19, 2024 14:39

jmarrec approved these changes Dec 19, 2024

View reviewed changes

jmarrec mentioned this pull request Dec 19, 2024

[CI] Ubuntu 24.04 EpwFile / SQFile tests segfaulting #5332

Closed

joseph-robertson merged commit 73e12b9 into develop Dec 19, 2024
2 checks passed

joseph-robertson deleted the python-plugin-search-paths branch December 19, 2024 17:19

jmarrec mentioned this pull request Apr 2, 2025

Fix C# bindings - unbreak the build and add missing partial classes #5377

Merged

19 tasks

jmarrec added a commit that referenced this pull request Apr 2, 2025

Fix C# bindings for #5312

f50acda

		bool addSearchPath(const std::string& searchPath);

		bool setSearchPaths(const std::vector<std::string>& searchPaths);

		/** PythonPluginSearchPaths is a ParentObject that wraps the OpenStudio IDD object 'OS:PythonPlugin:SearchPaths'. */
		class MODEL_API PythonPluginSearchPaths : public ParentObject


		bool addepinEnvironmentVariabletoSearchPath() const;

		std::vector<openstudio::path> searchPaths() const;

Conversation

joseph-robertson commented Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

jmarrec commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmarrec commented Dec 12, 2024

Uh oh!

jmarrec left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarrec commented Dec 19, 2024

Uh oh!

ci-commercialbuildings commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joseph-robertson commented Dec 2, 2024 •

edited

Loading

jmarrec commented Dec 12, 2024 •

edited

Loading

ci-commercialbuildings commented Dec 19, 2024 •

edited

Loading