OSArgument of type Path: incomplete and confusing implementation

[jmarrec]

Issue overview

The OS SDK and BCL-gem implementation of the OSArgumentType::Path arguments is incomplete, and neither handle the makePathArgument arguments:

OpenStudio/src/measure/OSArgument.hpp

Lines 131 to 133 in f63cae5

	/** Creates an OSArgument for path values. Defaults domainType() to OSDomainType::Enumeration. */
	static OSArgument makePathArgument(const std::string& name, bool isRead, const std::string& extension, bool required = true,
	bool modelDependent = false);

• isRead
  • what does that even mean?
  • Is this assuming that this is for reading meaning it must be 1) a file (not a directory) and 2) it must exist?)
• extension

Current Behavior

They are confusingly named, and not written to the measure.xml

Expected Behavior

Clarify the meaning of these. And ensure they get written to the measure.xml

Steps to Reproduce

here is an example measure to play with.

example_path_argument.zip

openstudio measure -u . produces a measure.xml with no extension or is_read.

Possible Solution

Ideally I think there should be 3 possible types of Path Arguments:

• A directory, no extensions specified
• A file that will be read and must exist. The extensions should be optional
• A file that will be written and does not have to exist. The extensions should be optional

I think the extensions should rather be specified as a kind of std::map<std::string, std::string>. And should be sanitized to a common format (eg check there is / there isn't a leading dot . and /or * or not) . for eg

// Display Name, extensions
"CSV Files", "*.csv"
"Excel Files", "*.xlsx *.xls"

Or maybe even a std::map<std::string, std::vector<std::string>>, such as an entry would be {"Excel Files", {"xlsx", "xls"}})

We must be able to differentiate from a Path of type directory and a Path of type file. Several options:

1. Make a new OSArgumentType::DirectoryPath
2. Ensure the extension is always set for the file ones, eg:

static OSArgument makePathArgument(const std::string& name, bool openForReading, const std::map<std::string, std::string>& extensions, bool required = true) {
   if (extensions.empty()) {
     m_extensions = {"All Files", "*"};
   } else {
     m_extensions = extensions;
  }
}


static OSArgument makeDirectoryArgument(const std::string& name, bool required = true) {
  m_extensions.clear()
} 

3. Add a bool isDirectory on the OSArgument class and modify the makePathArgument to take it in.

A potential measure.xml would look like

<arguments>
    <argument>
      <name>output_path</name>
      <display_name>Output Path on Disk</display_name>
      <type>Path</type>
      <required>true</required>
      <model_dependent>false</model_dependent>
      <is_read>true</is_read>
+     <is_directory>false</is_directory>     // May not be needed?
+     <extensions>
+       <extension>
+         <description>Excel Files</description>
+         <ext>xlsx</ext>
+         <ext>xls</ext>
+       </extension>
+       <extension>
+         <description>All files</description>
+         <ext></ext>
+       </extension>
+     </extensions>
    </argument>
  </arguments>

Further areas of improvements

If an existing file is what's wanted, the validateUserArguments method should probably catch the case where it doesn't exist.

Details

Environment

Some additional details about your environment for this issue (if relevant):

• Platform (Operating system, version): all
• Version of OpenStudio (if using an intermediate build, include SHA): 3.9.0 alpha

Context

Trying to implement it in OS Application at openstudiocoalition/OpenStudioApplication#761