Repository: NREL/OpenStudio (language: python)

Issue:
Add native support for measure argument methods in OsLib_HelperMethods
Pull request overview
---------------------

 - Addresses #4993.

Checklist:
- [x] `createRunVariables`
  - -> OSRunner: `getArgumentValues(script_arguments, user_arguments)`
- [x] `check_upstream_measure_for_arg` Should be pretty straightforward; we do this for ResStock often.
  - -> OSRunner: `getPastStepValuesForMeasure(measure_name)`
  - -> OSRunner: `getPastStepValuesForName(step_name)`
- [ ] ~`checkDoubleAndIntegerArguments` -> This may be already handled through new `setMin` and `setMax`. https://github.com/NREL/OpenStudio/issues/4551~
- [ ] `checkChoiceArgFromModelObjects` -> ?
- [ ] `checkOptionalChoiceArgFromModelObjects` -> ?
  - -> runner.checkArgumentValueExists(argument_name, user_arguments)
  - -> runner.checkArgumentValuesFromModelObjects(script_arguments, user_arguments)
- [ ] `populateChoiceArgFromModelObjects` -> Need to think about where something like this should live.
- [ ] `getAbsoluteAzimuthForSurface` -> ?

Questions:
- Return a `std::map<std::string, std::string>` hash? One implication is that we need to, e.g., `std::to_string` on doubles.
- For optional arguments that aren't assigned a value, should the hash contain the argument name as a key (with uninitialized value)?
- Is it possible to return the keys as symbols (instead of strings)?
- What is return type `Json::Value`? Does that have any relevance here?
  - Answer: `Json::Value` seems to be almost exactly what we want to use here. Except that for optional arguments with no value assigned, the returned hash will have `:key=>nil` instead of `:key=>boost::none`. Maybe there's a way to return `boost::none`, but either way I would think this would work.

### Pull Request Author

<!--- Add to this list or remove from it as applicable.  This is a simple templated set of guidelines. -->

 - [ ] 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: Add Link
 - [ ] 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

Off by one error in measure step_values when _SKIP_ is true
Issue overview
--------------
When adding measures to a workflow, there is a special _SKIP_ argument that can be used to skip running a measure.  Setting _SKIP_ to true results in step_values being added to the wrong measure in the out.osw file for subsequent measures in the workflow.

## Current Behavior
step_values are associated with the wrong measure in out.osw when a measure is skipped

## Expected Behavior
step_values should be associated with the correct measure in out.osw when a measure is skipped

## Steps to Reproduce
1. Unzip [MixedUpStepsSkip.zip](https://github.com/NREL/OpenStudio/files/14924550/MixedUpStepsSkip.zip)
2. openstudio.exe run -w workflow.osw
3. The step_results should be associated with the correct measure indexes but they are off by one when a measure is skipped
4. Additionally, `[openstudio.measure.OSRunner] <Error> Not prepared for step` is shown when measure is skipped

## Possible Solution

## Details

### Environment

Some additional details about your environment for this issue (if relevant):
 - Platform (Operating system, version): Windows 11
 - Version of OpenStudio (if using an intermediate build, include SHA): 3.7.0

### Context
Parsing of results in out.osw file broken for test suite


Discussion / hints:
CI Results for a94430438f0a3adf3dee29c2295fde4ffa3af379:

* Ubuntu 20.04:
* Ubuntu 22.04:
    * [OpenStudio-3.8.0+a94430438f-Ubuntu-22.04-x86_64.deb](http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/PR-5089/OpenStudio-3.8.0%2Ba94430438f-Ubuntu-22.04-x86_64.deb)
    * [OpenStudio-3.8.0+a94430438f-Ubuntu-22.04-x86_64.tar.gz](http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/PR-5089/OpenStudio-3.8.0%2Ba94430438f-Ubuntu-22.04-x86_64.tar.gz)

* Darwin:
    * [OpenStudio-3.8.0+a94430438f-Darwin-x86_64.dmg](http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/PR-5089/OpenStudio-3.8.0+a94430438f-Darwin-x86_64.dmg)
    * [OpenStudio-3.8.0+a94430438f-Darwin-x86_64.tar.gz](http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/PR-5089/OpenStudio-3.8.0+a94430438f-Darwin-x86_64.tar.gz)

* Windows:
    * [OpenStudio-3.8.0+a94430438f-Windows.exe](http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/PR-5089/OpenStudio-3.8.0+a94430438f-Windows.exe)
    * [OpenStudio-3.8.0+a94430438f-Windows.tar.gz](http://openstudio-ci-builds.s3-website-us-west-2.amazonaws.com/PR-5089/OpenStudio-3.8.0+a94430438f-Windows.tar.gz)

* Code Coverage:
I think https://github.com/NREL/OpenStudio/pull/5135 should supersede this PR.
This does not affect the `classic` CLI.

Fix patch:
diff --git a/src/measure/OSArgument.cpp b/src/measure/OSArgument.cpp
index e5cab442d70..60763def661 100644
--- a/src/measure/OSArgument.cpp
+++ b/src/measure/OSArgument.cpp
@@ -34,8 +34,8 @@ namespace measure {
   OSArgument OSArgument::makeBoolArgument(const std::string& name, bool required, bool modelDependent) {
     OSArgument result(name, OSArgumentType::Boolean, required, modelDependent);
     result.setDomainType(OSDomainType::Enumeration);
-    result.m_choices.push_back("true");
-    result.m_choices.push_back("false");
+    result.m_choices.emplace_back("true");
+    result.m_choices.emplace_back("false");
     return result;
   }
 
@@ -132,10 +132,7 @@ namespace measure {
       LOG_AND_THROW("This argument is of type " << type().valueName() << ", not of type Bool.");
     }
 
-    // Note JM 2019-05-17: This is functionally equivalent to `std::get<bool>(m_value)` except it doesn't risk throwing
-    // std::bad_variant_access which isn't available on mac prior to 10.14
-    // No need to check if get_if succeeds because we checked the type above
-    return *(std::get_if<bool>(&m_value));
+    return std::get<bool>(m_value);
   }
 
   double OSArgument::valueAsDouble() const {
@@ -146,11 +143,11 @@ namespace measure {
       LOG_AND_THROW("This argument is of type " << type().valueName() << ", not of type Double.");
     }
 
-    double result;
+    double result = 0.0;
     if (type() == OSArgumentType::Double) {
-      result = *(std::get_if<double>(&m_value));
+      result = std::get<double>(m_value);
     } else {
-      result = *(std::get_if<int>(&m_value));
+      result = std::get<int>(m_value);
       LOG(Warn, "This argument is of type 'Integer' but returning as a Double as requested. You should consider using valueAsInteger instead");
     }
 
@@ -165,7 +162,7 @@ namespace measure {
       LOG_AND_THROW("This argument is of type " << type().valueName() << ", not of type Integer.");
     }
 
-    return *(std::get_if<int>(&m_value));
+    return std::get<int>(m_value);
   }
 
   std::string OSArgument::valueAsString() const {
@@ -184,7 +181,11 @@ namespace measure {
       LOG_AND_THROW("This argument is of type " << type().valueName() << ", not of type Path.");
     }
 
-    return *(std::get_if<openstudio::path>(&m_value));
+    return std::get<openstudio::path>(m_value);
+  }
+
+  Json::Value OSArgument::valueAsJSON() const {
+    return argumentVariantToJSONValue(m_value);
   }
 
   bool OSArgument::hasDefaultValue() const {
@@ -199,7 +200,7 @@ namespace measure {
       LOG_AND_THROW("This argument is of type " << type().valueName() << ", not of type Bool.");
     }
 
-    return *(std::get_if<bool>(&m_defaultValue));
+    return std::get<bool>(m_defaultValue);
   }
 
   double OSArgument::defaultValueAsDouble() const {
@@ -210,7 +211,7 @@ namespace measure {
       LOG_AND_THROW("This argument is of type " << type().valueName() << ", not of type Double.");
     }
 
-    return *(std::get_if<double>(&m_defaultValue));
+    return std::get<double>(m_defaultValue);
   }
 
   int OSArgument::defaultValueAsInteger() const {
@@ -221,7 +222,7 @@ namespace measure {
       LOG_AND_THROW("This argument is of type " << type().valueName() << ", not of type Integer.");
     }
 
-    return *(std::get_if<int>(&m_defaultValue));
+    return std::get<int>(m_defaultValue);
   }
 
   std::string OSArgument::defaultValueAsString() const {
@@ -240,7 +241,11 @@ namespace measure {
       LOG_AND_THROW("This argument is of type " << type().valueName() << ", not of type Path.");
     }
 
-    return *(std::get_if<openstudio::path>(&m_defaultValue));
+    return std::get<openstudio::path>(m_defaultValue);
+  }
+
+  Json::Value OSArgument::defaultValueAsJSON() const {
+    return argumentVariantToJSONValue(m_value);
   }
 
   bool OSArgument::hasDomain() const {
@@ -260,9 +265,9 @@ namespace measure {
     }
 
     std::vector<bool> result;
-
+    result.reserve(m_domain.size());
     for (const OSArgumentVariant& value : m_domain) {
-      result.push_back(*(std::get_if<bool>(&value)));
+      result.push_back(std::get<bool>(value));
     }
     return result;
   }
@@ -279,14 +284,14 @@ namespace measure {
 
     if (type() == OSArgumentType::Double) {
       for (const OSArgumentVariant& value : m_domain) {
-        result.push_back(*(std::get_if<double>(&value)));
+        result.push_back(std::get<double>(value));
       }
     } else {
       // It's an int
       LOG(Warn, "This argument is of type 'Integer' but returning Domain as a Double as requested. "
                 "You should consider using domainAsInteger instead");
       for (const OSArgumentVariant& value : m_domain) {
-        result.push_back(*(std::get_if<int>(&value)));
+        result.push_back(std::get<int>(value));
       }
     }
     return result;
@@ -301,9 +306,9 @@ namespace measure {
     }
 
     std::vector<int> result;
-
+    result.reserve(m_domain.size());
     for (const OSArgumentVariant& value : m_domain) {
-      result.push_back(*(std::get_if<int>(&value)));
+      result.push_back(std::get<int>(value));
     }
     return result;
   }
@@ -317,9 +322,9 @@ namespace measure {
     }
 
     std::vector<openstudio::path> result;
-
+    result.reserve(m_domain.size());
     for (const OSArgumentVariant& value : m_domain) {
-      result.push_back(*(std::get_if<openstudio::path>(&value)));
+      result.push_back(std::get<openstudio::path>(value));
     }
     return result;
   }
@@ -331,13 +336,26 @@ namespace measure {
 
     // TODO: add check for arg type?
     std::vector<std::string> result;
-
+    result.reserve(m_domain.size());
     for (const OSArgumentVariant& value : m_domain) {
       result.push_back(printOSArgumentVariant(value));
     }
     return result;
   }
 
+  Json::Value OSArgument::domainAsJSON() const {
+    if (!hasDomain()) {
+      return Json::nullValue;
+    }
+
+    Json::Value root(Json::arrayValue);
+    for (const OSArgumentVariant& value : m_domain) {
+      root.append(argumentVariantToJSONValue(value));
+    }
+
+    return root;
+  }
+
   std::vector<std::string> OSArgument::choiceValues() const {
     return m_choices;
   }
@@ -348,7 +366,7 @@ namespace measure {
 
   std::string OSArgument::valueDisplayName() const {
     std::string valueString = valueAsString();
-    int index = int(std::find(m_choices.begin(), m_choices.end(), valueString) - m_choices.begin());
+    const int index = int(std::find(m_choices.begin(), m_choices.end(), valueString) - m_choices.begin());
     if (index < int(m_choiceDisplayNames.size())) {
       return m_choiceDisplayNames[index];
     }
@@ -357,7 +375,7 @@ namespace measure {
 
   std::string OSArgument::defaultValueDisplayName() const {
     std::string defaultValueString = defaultValueAsString();
-    int index = int(std::find(m_choices.begin(), m_choices.end(), defaultValueString) - m_choices.begin());
+    const int index = int(std::find(m_choices.begin(), m_choices.end(), defaultValueString) - m_choices.begin());
     if (index < int(m_choiceDisplayNames.size())) {
       return m_choiceDisplayNames[index];
     }
@@ -408,7 +426,7 @@ namespace measure {
 
     } else if (m_type == OSArgumentType::Integer) {
       // Let a double be assigned to an int, only if said double is really an integer expressed as a double
-      int test = (int)floor(value);
+      const int test = (int)floor(value);
       if (test == value) {
         result = setValue(test);
       } else {
@@ -455,7 +473,7 @@ namespace measure {
   }
 
   bool OSArgument::setValue(const std::string& value) {
-    bool result = setStringInternal(m_value, value);
+    const bool result = setStringInternal(m_value, value);
     if (result) {
       OS_ASSERT(hasValue());
       onChange();
@@ -489,7 +507,7 @@ namespace measure {
       result = true;
     } else if (m_type == OSArgumentType::Integer) {
       // Let a double be assigned to an int, only if said double is really an integer expressed as a double
-      int test = (int)floor(defaultValue);
+      const int test = (int)floor(defaultValue);
       if (test == defaultValue) {
         result = setDefaultValue(test);
       } else {
@@ -535,7 +553,7 @@ namespace measure {
   }
 
   bool OSArgument::setDefaultValue(const std::string& defaultValue) {
-    bool result = setStringInternal(m_defaultValue, defaultValue);
+    const bool result = setStringInternal(m_defaultValue, defaultValue);
     if (result) {
       OS_ASSERT(hasDefaultValue());
       onChange();
@@ -581,7 +599,7 @@ namespace measure {
       // could check for uniqueness, but pass on that for now
       m_domain.clear();
       for (bool value : domain) {
-        m_domain.push_back(OSArgumentVariant(value));
+        m_domain.emplace_back(value);
       }
       onChange();
       result = true;
@@ -596,7 +614,7 @@ namespace measure {
         // could check for uniqueness, min < max, but pass on that for now
         m_domain.clear();
         for (double value : domain) {
-          m_domain.push_back(OSArgumentVariant(value));
+          m_domain.emplace_back(value);
         }
         onChange();
         result = true;
@@ -612,7 +630,7 @@ namespace measure {
         // could check for uniqueness, min < max, but pass on that for now
         m_domain.clear();
         for (int value : domain) {
-          m_domain.push_back(OSArgumentVariant(value));
+          m_domain.emplace_back(value);
         }
         onChange();
         result = true;
@@ -622,7 +640,7 @@ namespace measure {
         // could check for uniqueness, min < max, but pass on that for now
         m_domain.clear();
         for (int value : domain) {
-          m_domain.push_back(OSArgumentVariant(double(value)));
+          m_domain.emplace_back(double(value));
         }
         onChange();
         result = true;
@@ -638,7 +656,7 @@ namespace measure {
       // could check for uniqueness, but pass on that for now
       m_domain.clear();
       for (const openstudio::path& value : domain) {
-        m_domain.push_back(OSArgumentVariant(value));
+        m_domain.emplace_back(value);
       }
       onChange();
       result = true;
@@ -650,7 +668,7 @@ namespace measure {
     bool result(false);
     if ((m_domainType != OSDomainType::Interval) || (domain.size() == 2u)) {
       // Store the original, in case we fail to set one element
-      std::vector<OSArgumentVariant> originalDomain = m_domain;
+      const std::vector<OSArgumentVariant> originalDomain = m_domain;
       m_domain.clear();
       for (const std::string& value : domain) {
         OSArgumentVariant newValue;
@@ -673,9 +691,9 @@ namespace measure {
     m_domain.clear();
   }
 
-  bool OSArgument::setMinValue(double minValue) {
+  bool OSArgument::setMinValue(double minValue) {  // NOLINT(misc-no-recursion)
     if (m_type == OSArgumentType::Integer) {
-      int test = (int)floor(minValue);
+      const int test = (int)floor(minValue);
       if (test == minValue) {
         // If int expressed as double (eg: 1.0 when type = int)
         // Then we call the int overload instead
@@ -697,15 +715,15 @@ namespace measure {
 
     m_domainType = OSDomainType::Interval;
     m_domain.clear();
-    m_domain.push_back(OSArgumentVariant(minValue));
-    m_domain.push_back(OSArgumentVariant(maxValue));
+    m_domain.emplace_back(minValue);
+    m_domain.emplace_back(maxValue);
 
     onChange();
 
     return true;
   }
 
-  bool OSArgument::setMinValue(int minValue) {
+  bool OSArgument::setMinValue(int minValue) {  // NOLINT(misc-no-recursion)
     if (m_type == OSArgumentType::Double) {
       auto test = (double)minValue;
       return setMinValue(test);
@@ -723,17 +741,17 @@ namespace measure {
 
     m_domainType = OSDomainType::Interval;
     m_domain.clear();
-    m_domain.push_back(OSArgumentVariant(minValue));
-    m_domain.push_back(OSArgumentVariant(maxValue));
+    m_domain.emplace_back(minValue);
+    m_domain.emplace_back(maxValue);
 
     onChange();
 
     return true;
   }
 
-  bool OSArgument::setMaxValue(double maxValue) {
+  bool OSArgument::setMaxValue(double maxValue) {  // NOLINT(misc-no-recursion)
     if (m_type == OSArgumentType::Integer) {
-      int test = (int)floor(maxValue);
+      const int test = (int)floor(maxValue);
       if (test == maxValue) {
         return setMaxValue(test);
       }
@@ -752,15 +770,15 @@ namespace measure {
 
     m_domainType = OSDomainType::Interval;
     m_domain.clear();
-    m_domain.push_back(OSArgumentVariant(minValue));
-    m_domain.push_back(OSArgumentVariant(maxValue));
+    m_domain.emplace_back(minValue);
+    m_domain.emplace_back(maxValue);
 
     onChange();
 
     return true;
   }
 
-  bool OSArgument::setMaxValue(int maxValue) {
+  bool OSArgument::setMaxValue(int maxValue) {  // NOLINT(misc-no-recursion)
     if (m_type == OSArgumentType::Double) {
       auto test = (double)maxValue;
       return setMaxValue(test);
@@ -778,23 +796,23 @@ namespace measure {
 
     m_domainType = OSDomainType::Interval;
     m_domain.clear();
-    m_domain.push_back(OSArgumentVariant(minValue));
-    m_domain.push_back(OSArgumentVariant(maxValue));
+    m_domain.emplace_back(minValue);
+    m_domain.emplace_back(maxValue);
 
     onChange();
 
     return true;
   }
 
-  bool OSArgument::setStringInternal(OSArgumentVariant& variant, const std::string& value) {
+  bool OSArgument::setStringInternal(OSArgumentVariant& argVar, const std::string& value) {
     bool result = false;
 
     if (m_type == OSArgumentType::Boolean) {
       if (openstudio::istringEqual(value, "true")) {
-        variant = true;
+        argVar = true;
         result = true;
       } else if (openstudio::istringEqual(value, "false")) {
-        variant = false;
+        argVar = false;
         result = true;
       } else {
         LOG(Debug, "Unknown value '" << value << "' for argument of type Bool.");
@@ -802,7 +820,7 @@ namespace measure {
     } else if (m_type == OSArgumentType::Double) {
       try {
         auto const double_val = std::stod(value, nullptr);
-        variant = double_val;
+        argVar = double_val;
         result = true;
       } catch (std::exception&) {
         LOG(Debug, "Unable to convert value '" << value << "' to argument of type Double.");
@@ -810,25 +828,25 @@ namespace measure {
     } else if (m_type == OSArgumentType::Integer) {
       try {
         auto const int_val = std::stoi(value, nullptr);
-        variant = int_val;
+        argVar = int_val;
         result = true;
       } catch (std::exception&) {
         LOG(Debug, "Unable to convert value '" << value << "' to argument of type Integer.");
       }
     } else if (m_type == OSArgumentType::String) {
-      variant = value;
+      argVar = value;
       result = true;
     } else if (m_type == OSArgumentType::Choice) {
       if (std::find(m_choices.begin(), m_choices.end(), value) != m_choices.end()) {
-        variant = value;
+        argVar = value;
         result = true;
       } else {
         // can also set using display name
         auto it = std::find(m_choiceDisplayNames.begin(), m_choiceDisplayNames.end(), value);
         if (it != m_choiceDisplayNames.end()) {
-          int index = int(it - m_choiceDisplayNames.begin());
+          const int index = int(it - m_choiceDisplayNames.begin());
           if (index < int(m_choices.size())) {
-            variant = m_choices[index];
+            argVar = m_choices[index];
             result = true;
           }
         } else {
@@ -836,8 +854,8 @@ namespace measure {
         }
       }
     } else if (m_type == OSArgumentType::Path) {
-      openstudio::path temp = toPath(value);
-      variant = temp;
+      const openstudio::path temp = toPath(value);
+      argVar = temp;
       result = true;
     }
 
@@ -877,7 +895,7 @@ namespace measure {
 
     if (m_type.value() == OSArgumentType::Choice) {
       ss << "Choices:" << '\n';
-      int dnn = m_choiceDisplayNames.size();
+      const int dnn = m_choiceDisplayNames.size();
       for (int i = 0, n = m_choices.size(); i < n; ++i) {
         ss << "  " << m_choices[i];
         if ((i < dnn) && (!m_choiceDisplayNames[i].empty())) {
@@ -937,39 +955,52 @@ namespace measure {
     return os;
   }
 
-  std::string OSArgument::printOSArgumentVariant(const OSArgumentVariant& toPrint) const {
-    OS_ASSERT(toPrint.index() != 0);
+  std::string OSArgument::printOSArgumentVariant(const OSArgumentVariant& argVar) {
+    OS_ASSERT(argVar.index() != 0);
     std::stringstream ss;
 
     // We use std::visit, filtering out the case where it's monostate
     // Aside from monostate, every possible type is streamable
-    //std::visit(
-    //[&ss](const auto& val){
-    ////Needed to properly compare the types
-    //using T = std::remove_cv_t<std::remove_reference_t<decltype(val)>>;
-    //if constexpr (!std::is_same_v<T, std::monostate>) {
-    //ss << val;
-    //}
-    //},
-    //arg);
-
-    // Note JM 2019-05-17: std::visit is problematic on mac below 10.14, because it might throw std::bad_variant_access
-    // So we don't use it here. Same with std::get, so we use get_if instead
-    if (const auto* p = std::get_if<bool>(&toPrint)) {
-      ss << std::boolalpha << *p;
-    } else if (const auto* p = std::get_if<double>(&toPrint)) {
-      ss << *p;
-    } else if (const auto* p = std::get_if<int>(&toPrint)) {
-      ss << *p;
-    } else if (const auto* p = std::get_if<std::string>(&toPrint)) {
-      ss << *p;
-    } else if (const auto* p = std::get_if<openstudio::path>(&toPrint)) {
-      ss << *p;
-    }
+    std::visit(
+      [&ss](const auto& arg) {
+        // Needed to properly compare the types
+        using T = std::decay_t<decltype(arg)>;
+        if constexpr (!std::is_same_v<T, std::monostate>) {
+          ss << arg;
+        }
+      },
+      argVar);
 
     return ss.str();
   }
 
+  // helper constant for the visitor below so we static assert we didn't miss a type
+  template <class>
+  inline constexpr bool always_false_v = false;
+
+  Json::Value OSArgument::argumentVariantToJSONValue(const OSArgumentVariant& argVar) {
+    return std::visit(
+      [](auto&& arg) -> Json::Value {
+        using T = std::decay_t<decltype(arg)>;
+        if constexpr (std::is_same_v<T, std::monostate>) {
+          return Json::nullValue;
+        } else if constexpr (std::is_same_v<T, bool>) {  // NOLINT(bugprone-branch-clone)
+          return arg;
+        } else if constexpr (std::is_same_v<T, double>) {
+          return arg;
+        } else if constexpr (std::is_same_v<T, int>) {
+          return arg;
+        } else if constexpr (std::is_same_v<T, std::string>) {
+          return arg;
+        } else if constexpr (std::is_same_v<T, openstudio::path>) {
+          return openstudio::toString(arg);
+        } else {
+          static_assert(always_false_v<T>, "non-exhaustive visitor!");
+        }
+      },
+      argVar);
+  }
+
   OSArgument::OSArgument() : m_uuid(createUUID()), m_versionUUID(createUUID()) {}
 
   OSArgument::OSArgument(const std::string& name, const OSArgumentType& type, bool required, bool modelDependent)
@@ -979,8 +1010,7 @@ namespace measure {
       m_displayName(name),
       m_type(type),
       m_required(required),
-      m_modelDependent(modelDependent),
-      m_isRead(false) {}
+      m_modelDependent(modelDependent) {}
 
   void OSArgument::onChange() {
     m_versionUUID = createUUID();
@@ -988,9 +1018,11 @@ namespace measure {
 
   OSArgument makeChoiceArgumentOfWorkspaceObjects(const std::string& name, const IddObjectType& iddObjectType, const Workspace& workspace,
                                                   bool required) {
+
+    const std::vector<WorkspaceObject> objects = workspace.getObjectsByType(iddObjectType);
     std::vector<std::pair<std::string, std::string>> intermediate;
+    intermediate.reserve(objects.size());
 
-    std::vector<WorkspaceObject> objects = workspace.getObjectsByType(iddObjectType);
     for (const WorkspaceObject& object : objects) {
       std::string objectName;
       if (object.name()) {
@@ -998,12 +1030,12 @@ namespace measure {
       } else {
         objectName = object.iddObject().type().valueName();
       }
-      intermediate.push_back(std::pair<std::string, std::string>(toString(object.handle()), objectName));
+      intermediate.emplace_back(toString(object.handle()), objectName);
     }
 
     std::sort(intermediate.begin(), intermediate.end(), SecondOfPairLess<std::pair<std::string, std::string>>());
 
-    int n = intermediate.size();
+    const int n = intermediate.size();
     StringVector choices(n);
     StringVector displayNames(n);
     for (int i = 0; i < n; ++i) {
@@ -1016,16 +1048,17 @@ namespace measure {
 
   OSArgument makeChoiceArgumentOfWorkspaceObjects(const std::string& name, const std::string& referenceName, const Workspace& workspace,
                                                   bool required) {
-    std::vector<std::pair<std::string, std::string>> intermediate;
 
-    std::vector<WorkspaceObject> objects = workspace.getObjectsByReference(referenceName);
+    const std::vector<WorkspaceObject> objects = workspace.getObjectsByReference(referenceName);
+    std::vector<std::pair<std::string, std::string>> intermediate;
+    intermediate.reserve(objects.size());
     for (const WorkspaceObject& object : objects) {
-      intermediate.push_back(std::pair<std::string, std::string>(toString(object.handle()), object.name().get()));
+      intermediate.emplace_back(toString(object.handle()), object.name().get());
     }
 
     std::sort(intermediate.begin(), intermediate.end(), SecondOfPairLess<std::pair<std::string, std::string>>());
 
-    int n = intermediate.size();
+    const int n = intermediate.size();
     StringVector choices(n);
     StringVector displayNames(n);
     for (int i = 0; i < n; ++i) {
diff --git a/src/measure/OSArgument.hpp b/src/measure/OSArgument.hpp
index b4896a0182d..55aea7489c8 100644
--- a/src/measure/OSArgument.hpp
+++ b/src/measure/OSArgument.hpp
@@ -187,6 +187,10 @@ OPENSTUDIO_ENUM( OSDomainType,
    *  type() != OSArgumentType::Path. */
     openstudio::path valueAsPath() const;
 
+    /** Returns this argument's value as a Json::Value. Will convert nicely to ruby/python native types.
+      * Returns nullValue if it doesn't have one */
+    Json::Value valueAsJSON() const;
+
     /** Returns true if this argument's default value has been set. */
     bool hasDefaultValue() const;
 
@@ -210,6 +214,10 @@ OPENSTUDIO_ENUM( OSDomainType,
    *  hasDefaultValue() or if type() != OSArgumentType::Path. */
     openstudio::path defaultValueAsPath() const;
 
+    /** Returns this argument's default value as a Json::Value. Will convert nicely to ruby/python native types.
+      * Returns nullValue if it doesn't have one */
+    Json::Value defaultValueAsJSON() const;
+
     /** Returns true if this argument has a non-null domain. */
     bool hasDomain() const;
 
@@ -239,6 +247,10 @@ OPENSTUDIO_ENUM( OSDomainType,
    *  OSArgumentType::Path. */
     std::vector<openstudio::path> domainAsPath() const;
 
+    /** Returns this argument's domain as a Json::Value. Will convert nicely to ruby/python native types.
+      * Returns nullValue if it doesn't have one, Json::arrayValue otherwise */
+    Json::Value domainAsJSON() const;
+
     //@}
     /** @name Choice Argument Getters */
     //@{
@@ -400,9 +412,11 @@ OPENSTUDIO_ENUM( OSDomainType,
     // we add std::monostate to allow the variant to be empty basically
     using OSArgumentVariant = std::variant<std::monostate, bool, double, int, std::string, openstudio::path>;
 
-    bool setStringInternal(OSArgumentVariant& variant, const std::string& value);
+    bool setStringInternal(OSArgumentVariant& argVar, const std::string& value);
+
+    static std::string printOSArgumentVariant(const OSArgumentVariant& argVar);
 
-    std::string printOSArgumentVariant(const OSArgumentVariant& toPrint) const;
+    static Json::Value argumentVariantToJSONValue(const OSArgumentVariant& argVar);
 
     // This also OS App related
     void onChange();
@@ -414,15 +428,15 @@ OPENSTUDIO_ENUM( OSDomainType,
     boost::optional<std::string> m_description;
     OSArgumentType m_type;
     boost::optional<std::string> m_units;
-    bool m_required;
-    bool m_modelDependent;
+    bool m_required = false;
+    bool m_modelDependent = false;
     OSArgumentVariant m_value;
     OSArgumentVariant m_defaultValue;
     OSDomainType m_domainType;
     std::vector<OSArgumentVariant> m_domain;
     std::vector<std::string> m_choices;
     std::vector<std::string> m_choiceDisplayNames;
-    bool m_isRead;
+    bool m_isRead = false;
     std::string m_extension;
   };
 
diff --git a/src/measure/OSRunner.cpp b/src/measure/OSRunner.cpp
index 309caf9e376..926a59838e6 100644
--- a/src/measure/OSRunner.cpp
+++ b/src/measure/OSRunner.cpp
@@ -8,16 +8,15 @@
 #include "OSArgument.hpp"
 #include "OSMeasure.hpp"
 
+#include "../utilities/core/Assert.hpp"
+#include "../utilities/core/Containers.hpp"
+#include "../utilities/core/PathHelpers.hpp"
+#include "../utilities/core/StringHelpers.hpp"
+#include "../utilities/filetypes/WorkflowStep.hpp"
+#include "../utilities/filetypes/WorkflowStep_Impl.hpp"
 #include "../utilities/idf/Workspace.hpp"
 #include "../utilities/idf/WorkspaceObject.hpp"
 #include "../utilities/math/FloatCompare.hpp"
-#include "../utilities/filetypes/WorkflowStep.hpp"
-#include "../utilities/core/StringHelpers.hpp"
-
-#include "../utilities/core/Assert.hpp"
-#include "../utilities/core/PathHelpers.hpp"
-
-#include <boost/lexical_cast.hpp>
 
 #include <fmt/format.h>
 #include <cstdio>
@@ -183,14 +182,14 @@ namespace measure {
 
     // get list of new files
     if (m_currentDir) {
-      openstudio::path absoluteRootDir = m_workflow.absoluteRootDir();
+      const openstudio::path absoluteRootDir = m_workflow.absoluteRootDir();
       if (boost::filesystem::exists(*m_currentDir) && boost::filesystem::is_directory(*m_currentDir)) {
         for (boost::filesystem::directory_iterator dir_iter(*m_currentDir), end_iter; dir_iter != end_iter; ++dir_iter) {
           if (boost::filesystem::is_regular_file(dir_iter->status())) {
             if (m_currentDirFiles.find(dir_iter->path()) == m_currentDirFiles.end()) {
 
-              openstudio::path path = dir_iter->path();
-              OS_ASSERT(path.is_absolute());
+              const openstudio::path& fPath = dir_iter->path();
+              OS_ASSERT(fPath.is_absolute());
 
               // DLM: we need to figure out what these should be
               // are they absolute, relative to root dir, relative to measure dir, valid after reports have been packed up?
@@ -198,7 +197,7 @@ namespace measure {
               //  path = relativePath(path, absoluteRootDir);
               //} catch (const std::exception&){
               //}
-              m_result.addStepFile(path);
+              m_result.addStepFile(fPath);
             }
           }
         }
@@ -227,6 +226,10 @@ namespace measure {
   //}
 
   void OSRunner::prepareForMeasureRun(const OSMeasure& /*measure*/) {
+    prepareForMeasureRun();
+  }
+
+  void OSRunner::prepareForMeasureRun() {
     if (m_halted) {
       LOG(Error, "Wokflow halted, cannot prepate for measure run");
       return;
@@ -235,7 +238,7 @@ namespace measure {
       LOG(Error, "Step already started");
       return;
     }
-    boost::optional<WorkflowStep> currentStep = m_workflow.currentStep();
+    const boost::optional<WorkflowStep> currentStep = m_workflow.currentStep();
     if (!currentStep) {
       LOG(Error, "Cannot find current Workflow Step");
       return;
@@ -318,7 +321,7 @@ namespace measure {
   }
 
   void OSRunner::registerValue(const std::string& name, bool value) {
-    WorkflowStepValue stepValue(cleanValueName(name), value);
+    const WorkflowStepValue stepValue(cleanValueName(name), value);
     m_result.addStepValue(stepValue);
   }
 
@@ -329,7 +332,7 @@ namespace measure {
   }
 
   void OSRunner::registerValue(const std::string& name, double value) {
-    WorkflowStepValue stepValue(cleanValueName(name), value);
+    const WorkflowStepValue stepValue(cleanValueName(name), value);
     m_result.addStepValue(stepValue);
   }
 
@@ -353,7 +356,7 @@ namespace measure {
   }
 
   void OSRunner::registerValue(const std::string& name, int value) {
-    WorkflowStepValue stepValue(cleanValueName(name), value);
+    const WorkflowStepValue stepValue(cleanValueName(name), value);
     m_result.addStepValue(stepValue);
   }
 
@@ -377,7 +380,7 @@ namespace measure {
   }
 
   void OSRunner::registerValue(const std::string& name, const std::string& value) {
-    WorkflowStepValue stepValue(cleanValueName(name), value);
+    const WorkflowStepValue stepValue(cleanValueName(name), value);
     m_result.addStepValue(stepValue);
   }
 
@@ -412,7 +415,7 @@ namespace measure {
         }
       } else {
         // script_argument is in user_arguments
-        OSArgument user_argument = it->second;
+        const OSArgument& user_argument = it->second;
 
         // check that names still match
         if (user_argument.name() != script_argument.name()) {
@@ -787,60 +790,83 @@ namespace measure {
   }
 
   Json::Value OSRunner::getArgumentValues(std::vector<OSArgument>& script_arguments, const std::map<std::string, OSArgument>& user_arguments) {
+    // This function aims to replace OsLib_HelperMethods.createRunVariables
+    // TODO: should this call validateUserArguments like the ruby counterpart? At least it validates that the defaults are supposed to match etc
+    if (!validateUserArguments(script_arguments, user_arguments)) {
+      registerError("Invalid argument values.");
+      LOG_AND_THROW("Invalid argument values.");
+    }
+
     Json::Value argument_values;
-    std::string name;
     for (const OSArgument& script_argument : script_arguments) {
-      name = script_argument.name();
+      const std::string name = script_argument.name();
       auto it = user_arguments.find(name);
       if (it != user_arguments.end()) {
-        Json::Value root = it->second.toJSON();
-        if (auto value = root["value"]) {
-          argument_values[name] = value;
+        const auto& arg = it->second;
+        if (arg.hasValue()) {
+          argument_values[name] = arg.valueAsJSON();
+        } else if (arg.hasDefaultValue()) {
+          // TODO: is this wanted?
+          argument_values[name] = arg.defaultValueAsJSON();
         }
       }
     }
     return argument_values;
   }
 
-  Json::Value OSRunner::getPastStepValuesForMeasure(const std::string& measure_name) {
-    Json::Value step_values;
-    WorkflowJSON workflow_ = workflow();
-    std::vector<WorkflowStep> workflow_steps = workflow_.workflowSteps();
-    for (const WorkflowStep& workflow_step : workflow_steps) {
-      if (boost::optional<WorkflowStepResult> workflow_step_result_ = workflow_step.result()) {
-        boost::optional<std::string> measure_name_ = workflow_step_result_.get().measureName();
-        std::vector<WorkflowStepValue> workflow_step_values = workflow_step_result_.get().stepValues();
-        if (workflow_step_result_.get().value() == StepResult::Success) {
-          if (istringEqual(measure_name, measure_name_.get())) {
-            for (const WorkflowStepValue& workflow_step_value : workflow_step_values) {
-              Json::Value root = workflow_step_value.toJSON();
-              if (auto value = root["value"]) {
-                step_values[workflow_step_value.name()] = value;
-              }
-            }
-          }
+  Json::Value OSRunner::getPastStepValuesForMeasure(const std::string& measureName) const {
+    Json::Value step_values(Json::objectValue);
+    for (const MeasureStep& step : subsetCastVector<MeasureStep>(m_workflow.workflowSteps())) {
+      boost::optional<WorkflowStepResult> stepResult_ = step.result();
+      if (!stepResult_ || !stepResult_->stepResult() || stepResult_->stepResult().get() != StepResult::Success) {
+        continue;
+      }
+      // TODO: should we match on any of these three?
+      if (istringEqual(measureName, step.measureDirName())) {  // The directory name, eg `IncreaseWallRValue`
+        LOG(Trace, "Step matches on measureDirName");
+      } else if (auto s_ = step.name(); istringEqual(measureName, *s_)) {  // An optional, abritrary one
+        LOG(Trace, "Step matches on name");
+      } else if (auto s_ = stepResult_->measureName();
+                 istringEqual(measureName, *s_)) {  // The xml one, eg `increase_insulation_r_value_for_exterior_walls_by_percentage`
+        LOG(Trace, "Step matches on Step Result's measureName");
+      } else {
+        continue;
+      }
+
+      for (const WorkflowStepValue& step_value : stepResult_->stepValues()) {
+        Json::Value root = step_value.toJSON();
+        if (auto value = root["value"]) {
+          step_values[step_value.name()] = value;
         }
       }
+      break;
     }
     return step_values;
   }
 
-  Json::Value OSRunner::getPastStepValuesForName(const std::string& step_name) {
-    Json::Value step_values;
-    WorkflowJSON workflow_ = workflow();
-    std::vector<WorkflowStep> workflow_steps = workflow_.workflowSteps();
-    for (const WorkflowStep& workflow_step : workflow_steps) {
-      if (boost::optional<WorkflowStepResult> workflow_step_result_ = workflow_step.result()) {
-        boost::optional<std::string> measure_name_ = workflow_step_result_.get().measureName();
-        std::vector<WorkflowStepValue> workflow_step_values = workflow_step_result_.get().stepValues();
-        if (workflow_step_result_.get().value() == StepResult::Success) {
-          for (const WorkflowStepValue& workflow_step_value : workflow_step_values) {
-            if (istringEqual(step_name, workflow_step_value.name())) {
-              Json::Value root = workflow_step_value.toJSON();
-              if (auto value = root["value"]) {
-                step_values[measure_name_.get()] = value;
-              }
-            }
+  Json::Value OSRunner::getPastStepValuesForName(const std::string& stepName) const {
+    // This function aims to replace OsLib_HelperMethods.check_upstream_measure_for_arg
+    Json::Value step_values(Json::objectValue);
+    for (const MeasureStep& step : subsetCastVector<MeasureStep>(m_workflow.workflowSteps())) {
+      boost::optional<WorkflowStepResult> stepResult_ = step.result();
+      if (!stepResult_ || !stepResult_->stepResult() || stepResult_->stepResult().get() != StepResult::Success) {
+        continue;
+      }
+
+      std::string measure_name = step.measureDirName();  // The directory name, eg `IncreaseWallRValue`
+      if (auto s_ = step.name()) {                       // An optional, abritrary one
+        measure_name = std::move(*s_);
+      }
+      // if (auto s_ = stepResult_->measureName()) {  // The xml one, eg `increase_insulation_r_value_for_exterior_walls_by_percentage`
+      //   measure_name = std::move(*s_);
+      // }
+
+      for (const WorkflowStepValue& stepValue : stepResult_->stepValues()) {
+        if (istringEqual(stepName, stepValue.name())) {
+          Json::Value root = stepValue.toJSON();
+          if (auto value = root["value"]) {
+            step_values[measure_name] = value;
+            // TODO: check_upstream_measure_for_arg was breaking early, and returning a {:value => value, :measure_name => measure_name} format
           }
         }
       }
@@ -942,7 +968,7 @@ namespace measure {
     m_languagePreference = "en";
   }
 
-  std::string OSRunner::cleanValueName(const std::string& name) const {
+  std::string OSRunner::cleanValueName(const std::string& name) {
     static const boost::regex allowableCharacters("[^0-9a-zA-Z]");
     static const boost::regex leadingUnderscore("^_+");
     static const boost::regex trailingUnderscore("_+$");
diff --git a/src/measure/OSRunner.hpp b/src/measure/OSRunner.hpp
index b0d8b89da5c..37db45f5f44 100644
--- a/src/measure/OSRunner.hpp
+++ b/src/measure/OSRunner.hpp
@@ -19,6 +19,7 @@
 
 namespace openstudio {
 
+class OSWorkflow;
 class Workspace;
 class WorkspaceObject;
 
@@ -231,14 +232,16 @@ namespace measure {
                                                                                        const std::map<std::string, OSArgument>& user_arguments,
                                                                                        const openstudio::Workspace& workspace);
 
-    /** Call this method to retrieve the values of OSArguments of all types. */
+    /** Call this method to retrieve the values of OSArguments of all types.
+      * It will call validateUserArguments for you and throw if that fails */
     Json::Value getArgumentValues(std::vector<OSArgument>& script_arguments, const std::map<std::string, OSArgument>& user_arguments);
 
-    /** Call this method to retrieve the values of all workflow steps for a given measure name. */
-    Json::Value getPastStepValuesForMeasure(const std::string& measure_name);
+    /** Call this method to retrieve the values of all workflow steps for a given measure name.
+     *  measureName can be: the WorkflowStep's measureDirName or name, or the WorkflowStepResult's measureName */
+    Json::Value getPastStepValuesForMeasure(const std::string& measureName) const;
 
-    /** Call this method to retrieve the values of all workflow steps for a given step name. */
-    Json::Value getPastStepValuesForName(const std::string& step_name);
+    /** Call this method to retrieve the values of all workflow steps for a given step value name. */
+    Json::Value getPastStepValuesForName(const std::string& stepName) const;
 
     //@}
 
@@ -288,10 +291,17 @@ namespace measure {
     bool registerMsgAlsoLogs() const;
     void setRegisterMsgAlsoLogs(bool registerMsgAlsoLogs);
 
+   protected:
+    friend class openstudio::OSWorkflow;
+    // prepareForMeasureRun(OSMeasure) actually doesn't use the OSMeasure, but it's a check that an OSMeasure is there.
+    // This is the actual implementation though, and it's protected by OSWorkflow is friended so it can call it before SKIPPING a step without the
+    // performance penatly of actually loading and resolving the measure just to skip it
+    void prepareForMeasureRun();
+
    private:
     REGISTER_LOGGER("openstudio.measure.OSRunner");
 
-    std::string cleanValueName(const std::string& name) const;
+    static std::string cleanValueName(const std::string& name);
 
     void captureStreams();
     void restoreStreams();
diff --git a/src/utilities/data/Attribute.cpp b/src/utilities/data/Attribute.cpp
index 5fe61d92625..4ead7c33c4b 100644
--- a/src/utilities/data/Attribute.cpp
+++ b/src/utilities/data/Attribute.cpp
@@ -119,7 +119,7 @@ namespace detail {
       m_value(value),
       m_units(units) {}
 
-  Attribute_Impl::Attribute_Impl(const pugi::xml_node& element) {
+  Attribute_Impl::Attribute_Impl(const pugi::xml_node& element) {  // NOLINT(misc-no-recursion)
     if (!element) {
       LOG_AND_THROW("Cannot construct Attribute from null pugi::xml_node element");
     }
@@ -230,8 +230,8 @@ namespace detail {
     } else if (m_valueType.value() == AttributeValueType::AttributeVector) {
       std::vector<Attribute> children;
 
-      for (pugi::xml_node& childElement : valueElement.children()) {
-        children.push_back(Attribute(childElement));
+      for (const pugi::xml_node& childElement : valueElement.children()) {
+        children.emplace_back(childElement);
       }
       m_value = children;
 
@@ -486,14 +486,14 @@ namespace detail {
     return ss.str();
   }
 
-  pugi::xml_document Attribute_Impl::toXml() const {
+  pugi::xml_document Attribute_Impl::toXml() const {  // NOLINT(misc-no-recursion)
     pugi::xml_document doc;
     pugi::xml_node element = doc.append_child("Attribute");
     this->writeValues(element);
     return doc;
   }
 
-  bool Attribute_Impl::operator==(const Attribute& other) const {
+  bool Attribute_Impl::operator==(const Attribute& other) const {  // NOLINT(misc-no-recursion)
     bool result = false;
 
     AttributeValueType thisValueType = this->valueType();
@@ -541,7 +541,7 @@ namespace detail {
     return result;
   }
 
-  void Attribute_Impl::writeValues(pugi::xml_node& element) const {
+  void Attribute_Impl::writeValues(pugi::xml_node& element) const {  // NOLINT(misc-no-recursion)
     pugi::xml_node subElement = element.append_child("UUID");
     subElement.text().set(openstudio::toString(m_uuid).c_str());
 
@@ -604,7 +604,7 @@ namespace detail {
     }
   }
 
-  Json::Value Attribute_Impl::toJSON(bool short_version) const {
+  Json::Value Attribute_Impl::toJSON(bool short_version) const {  // NOLINT(misc-no-recursion)
 
     Json::Value root;
     root["name"] = m_name;
@@ -656,6 +656,65 @@ namespace detail {
     return root;
   }
 
+  // helper constant for the visitor below so we static assert we didn't miss a type
+  template <class>
+  inline constexpr bool always_false_v = false;
+
+#if 0
+  // helper type for visitor
+  template <class... Ts>
+  struct overloaded : Ts...
+  {
+    using Ts::operator()...;
+  };
+  // explicit deduction guide (not needed as of C++20)
+  template <class... Ts>
+  overloaded(Ts...) -> overloaded<Ts...>;
+
+  Json::Value Attribute_Impl::valueAsJSON() const {
+    return std::visit(overloaded{//
+                                 [](std::monostate) { return Json::Value{Json::nullValue}; },
+                                 [](const std::vector<Attribute>& arg) {
+                                   Json::Value subElement(Json::arrayValue);
+                                   for (const Attribute& attribute : arg) {
+                                     subElement.append(attribute.toJSON());
+                                   }
+                                   return subElement;
+                                 },
+                                 [](auto arg) { return Json::Value(arg); }},
+                      m_value);
+  }
+#endif
+
+  Json::Value Attribute_Impl::valueAsJSON() const {  // NOLINT(misc-no-recursion)
+    return std::visit(
+      [](auto&& arg) -> Json::Value {
+        using T = std::decay_t<decltype(arg)>;
+        if constexpr (std::is_same_v<T, std::monostate>) {
+          return Json::nullValue;
+        } else if constexpr (std::is_same_v<T, bool>) {  // NOLINT(bugprone-branch-clone)
+          return arg;
+        } else if constexpr (std::is_same_v<T, double>) {
+          return arg;
+        } else if constexpr (std::is_same_v<T, int>) {
+          return arg;
+        } else if constexpr (std::is_same_v<T, unsigned int>) {
+          return arg;
+        } else if constexpr (std::is_same_v<T, std::string>) {
+          return arg;
+        } else if constexpr (std::is_same_v<T, std::vector<Attribute>>) {
+          Json::Value subElement(Json::arrayValue);
+          for (const Attribute& attribute : arg) {
+            subElement.append(attribute.valueAsJSON());
+          }
+          return subElement;
+        } else {
+          static_assert(always_false_v<T>, "non-exhaustive visitor!");
+        }
+      },
+      m_value);
+  }
+
 }  // namespace detail
 
 // For debug purposes only
@@ -1031,11 +1090,11 @@ std::string Attribute::toString() const {
 }
 
 // Protected
-pugi::xml_document Attribute::toXml() const {
+pugi::xml_document Attribute::toXml() const {  // NOLINT(misc-no-recursion)
   return m_impl->toXml();
 }
 
-Json::Value Attribute::toJSON(bool short_version) const {
+Json::Value Attribute::toJSON(bool short_version) const {  // NOLINT(misc-no-recursion)
   return m_impl->toJSON(short_version);
 }
 
@@ -1043,6 +1102,10 @@ std::string Attribute::toJSONString() const {
   return toJSON().toStyledString();
 }
 
+Json::Value Attribute::valueAsJSON() const {
+  return m_impl->valueAsJSON();
+}
+
 bool Attribute::saveToXml(const openstudio::path& path) const {
   bool result = false;
 
@@ -1058,7 +1121,7 @@ bool Attribute::saveToXml(const openstudio::path& path) const {
   return result;
 }
 
-bool Attribute::operator==(const Attribute& other) const {
+bool Attribute::operator==(const Attribute& other) const {  // NOLINT(misc-no-recursion)
   return m_impl->operator==(other);
 }
 
diff --git a/src/utilities/data/Attribute.hpp b/src/utilities/data/Attribute.hpp
index 3ce5bb9b0b8..1f4a17e74a1 100644
--- a/src/utilities/data/Attribute.hpp
+++ b/src/utilities/data/Attribute.hpp
@@ -304,6 +304,9 @@ class UTILITIES_API Attribute
   Json::Value toJSON(bool short_version = true) const;
   std::string toJSONString() const;
 
+  // Just the value as a Json::Value
+  Json::Value valueAsJSON() const;
+
  protected:
   UTILITIES_API friend std::ostream& operator<<(std::ostream& os, const Attribute& attribute);
   // TODO: remove from public API or make protected/private
diff --git a/src/utilities/data/Attribute_Impl.hpp b/src/utilities/data/Attribute_Impl.hpp
index 05973e686a7..9d1d8c0c8ca 100644
--- a/src/utilities/data/Attribute_Impl.hpp
+++ b/src/utilities/data/Attribute_Impl.hpp
@@ -166,6 +166,9 @@ namespace detail {
     // short_version: if false, will include the uuid, versionuuid and source (if any)
     Json::Value toJSON(bool short_version) const;
 
+    // Just the value as a Json::Value
+    Json::Value valueAsJSON() const;
+
     /// comparison
     bool operator==(const Attribute& other) const;
 
diff --git a/src/utilities/data/Test/Attribute_GTest.cpp b/src/utilities/data/Test/Attribute_GTest.cpp
index 32d57052df2..70c4845cd63 100644
--- a/src/utilities/data/Test/Attribute_GTest.cpp
+++ b/src/utilities/data/Test/Attribute_GTest.cpp
@@ -12,6 +12,7 @@
 
 #include <boost/regex.hpp>
 
+#include <json/value.h>
 #include <pugixml.hpp>
 #include <sstream>
 #include <limits>
@@ -32,6 +33,11 @@ TEST_F(DataFixture, Attribute_BoolTrue) {
   EXPECT_EQ(true, attribute.valueAsBoolean());
   ASSERT_TRUE(attribute.units());
   EXPECT_EQ("units", attribute.units().get());
+  {
+    auto boolJson = attribute.valueAsJSON();
+    ASSERT_TRUE(boolJson.isBool());
+    EXPECT_TRUE(boolJson.asBool());
+  }
 
   // save to xml
   ASSERT_TRUE(attribute.saveToXml(xmlPath));
@@ -57,6 +63,11 @@ TEST_F(DataFixture, Attribute_Integer) {
   EXPECT_EQ(AttributeValueType::Integer, attribute.valueType().value());
   EXPECT_EQ(1, attribute.valueAsInteger());
   EXPECT_FALSE(attribute.units());
+  {
+    auto intJson = attribute.valueAsJSON();
+    ASSERT_TRUE(intJson.isInt());
+    EXPECT_EQ(1, intJson.asInt());
+  }
 
   // save to xml
   attribute.saveToXml(xmlPath);
@@ -81,7 +92,11 @@ TEST_F(DataFixture, Attribute_Unsigned) {
   EXPECT_EQ(AttributeValueType::Unsigned, attribute.valueType().value());
   EXPECT_EQ(static_cast<unsigned>(1), attribute.valueAsUnsigned());
   EXPECT_FALSE(attribute.units());
-
+  {
+    auto uintJson = attribute.valueAsJSON();
+    ASSERT_TRUE(uintJson.isUInt());
+    EXPECT_EQ(1u, uintJson.asUInt());
+  }
   // save to xml
   attribute.saveToXml(xmlPath);
 
@@ -105,7 +120,11 @@ TEST_F(DataFixture, Attribute_Double_Small) {
   EXPECT_EQ(AttributeValueType::Double, attribute.valueType().value());
   EXPECT_EQ(1.5, attribute.valueAsDouble());
   EXPECT_FALSE(attribute.units());
-
+  {
+    auto doubleJson = attribute.valueAsJSON();
+    ASSERT_TRUE(doubleJson.isDouble());
+    EXPECT_DOUBLE_EQ(1.5, doubleJson.asDouble());
+  }
   // save to xml
   attribute.saveToXml(xmlPath);
 
@@ -129,7 +148,11 @@ TEST_F(DataFixture, Attribute_Double_Big) {
   EXPECT_EQ(AttributeValueType::Double, attribute.valueType().value());
   EXPECT_DOUBLE_EQ(1.189679819371987395175049501E32, attribute.valueAsDouble());
   EXPECT_FALSE(attribute.units());
-
+  {
+    auto doubleJson = attribute.valueAsJSON();
+    ASSERT_TRUE(doubleJson.isDouble());
+    EXPECT_DOUBLE_EQ(1.189679819371987395175049501E32, doubleJson.asDouble());
+  }
   // save to xml
   attribute.saveToXml(xmlPath);
 
@@ -166,7 +189,11 @@ TEST_F(DataFixture, Attribute_String) {
   EXPECT_EQ(AttributeValueType::String, attribute.valueType().value());
   EXPECT_EQ("value", attribute.valueAsString());
   EXPECT_FALSE(attribute.units());
-
+  {
+    auto stringJson = attribute.valueAsJSON();
+    ASSERT_TRUE(stringJson.isString());
+    EXPECT_EQ("value", stringJson.asString());
+  }
   // save to xml
   attribute.saveToXml(xmlPath);
 
@@ -198,6 +225,22 @@ TEST_F(DataFixture, Attribute_AttributeVector) {
   EXPECT_EQ(AttributeValueType::Double, attribute.valueAsAttributeVector()[1].valueType().value());
   EXPECT_FALSE(attribute.units());
 
+  {
+    auto arrayJson = attribute.valueAsJSON();
+    ASSERT_TRUE(arrayJson.isArray());
+    ASSERT_EQ(2, arrayJson.size());
+    {
+      const auto& el = arrayJson[0];
+      ASSERT_TRUE(el.isBool());
+      EXPECT_TRUE(el.asBool());
+    }
+    {
+      const auto& el = arrayJson[1];
+      ASSERT_TRUE(el.isDouble());
+      EXPECT_DOUBLE_EQ(1.5, el.asDouble());
+    }
+  }
+
   // save to xml
   attribute.saveToXml(xmlPath);
 
diff --git a/src/utilities/data/Test/Variant_GTest.cpp b/src/utilities/data/Test/Variant_GTest.cpp
index ae3891ccdb6..40cc438ce37 100644
--- a/src/utilities/data/Test/Variant_GTest.cpp
+++ b/src/utilities/data/Test/Variant_GTest.cpp
@@ -8,6 +8,8 @@
 
 #include "../Variant.hpp"
 
+#include <json/value.h>
+
 using namespace openstudio;
 
 TEST_F(DataFixture, Variant) {
@@ -42,3 +44,33 @@ TEST_F(DataFixture, Variant) {
   EXPECT_THROW(stringVariant.valueAsInteger(), openstudio::Exception);
   EXPECT_THROW(stringVariant.valueAsDouble(), openstudio::Exception);
 }
+
+TEST_F(DataFixture, Variant_valueAsJSON) {
+  {
+    const Variant boolVariant(true);
+    auto boolJson = boolVariant.valueAsJSON();
+    ASSERT_TRUE(boolJson.isBool());
+    EXPECT_TRUE(boolJson.asBool());
+  }
+
+  {
+    const Variant intVariant(1);
+    auto intJson = intVariant.valueAsJSON();
+    ASSERT_TRUE(intJson.isInt());
+    EXPECT_EQ(1, intJson.asInt());
+  }
+
+  {
+    const Variant doubleVariant(1.0);
+    auto doubleJson = doubleVariant.valueAsJSON();
+    ASSERT_TRUE(doubleJson.isDouble());
+    EXPECT_DOUBLE_EQ(1, doubleJson.asDouble());
+  }
+
+  {
+    const Variant stringVariant("1");
+    auto stringJson = stringVariant.valueAsJSON();
+    ASSERT_TRUE(stringJson.isString());
+    EXPECT_EQ("1", stringJson.asString());
+  }
+}
diff --git a/src/utilities/data/Variant.cpp b/src/utilities/data/Variant.cpp
index ad7b00bc13c..c78cd2d3c21 100644
--- a/src/utilities/data/Variant.cpp
+++ b/src/utilities/data/Variant.cpp
@@ -4,6 +4,7 @@
 ***********************************************************************************************************************/
 
 #include "Variant.hpp"
+#include <json/value.h>
 
 namespace openstudio {
 
@@ -24,46 +25,81 @@ VariantType Variant::variantType() const {
 }
 
 bool Variant::valueAsBoolean() const {
-  // Note JM 2019-05-17: This is functionally equivalent to `std::get<bool>(m_value)` except it doesn't risk throwing
-  // std::bad_variant_access which isn't available on mac prior to 10.14
-  if (const auto* p = std::get_if<bool>(&m_value)) {
-    return *p;
-  } else {
+  if (m_type != VariantType::Boolean) {
     LOG_AND_THROW("Variant does not hold a boolean");
   }
-  return *(std::get_if<bool>(&m_value));
+  return std::get<bool>(m_value);
 }
 
 int Variant::valueAsInteger() const {
-  if (const auto* p = std::get_if<int>(&m_value)) {
-    return *p;
-  } else {
+  if (m_type != VariantType::Integer) {
     LOG_AND_THROW("Variant does not hold an int");
   }
+  return std::get<int>(m_value);
 }
 
 double Variant::valueAsDouble() const {
   if (m_type == VariantType::Integer) {
-    return (double)valueAsInteger();
+    return static_cast<double>(valueAsInteger());
   }
 
-  if (const auto* p = std::get_if<double>(&m_value)) {
-    return *p;
-  } else {
+  if (m_type != VariantType::Double) {
     LOG_AND_THROW("Variant does not hold a double");
   }
+  return std::get<double>(m_value);
 }
 
 std::string Variant::valueAsString() const {
-  if (const auto* p = std::get_if<std::string>(&m_value)) {
-    return *p;
-  } else {
+  if (m_type != VariantType::String) {
     LOG_AND_THROW("Variant does not hold a string");
   }
+  return std::get<std::string>(m_value);
+}
+
+// helper constant for the visitor below so we static assert we didn't miss a type
+template <class>
+inline constexpr bool always_false_v = false;
+
+Json::Value Variant::valueAsJSON() const {
+  return std::visit(
+    [](auto&& arg) -> Json::Value {
+      using T = std::decay_t<decltype(arg)>;
+      if constexpr (std::is_same_v<T, bool>) {  // NOLINT(bugprone-branch-clone)
+        return arg;
+      } else if constexpr (std::is_same_v<T, int>) {
+        return arg;
+      } else if constexpr (std::is_same_v<T, double>) {
+        return arg;
+      } else if constexpr (std::is_same_v<T, std::string>) {
+        return arg;
+      } else {
+        static_assert(always_false_v<T>, "non-exhaustive visitor!");
+      }
+    },
+    m_value);
+}
+
+bool Variant::isTrueish() const {
+  return std::visit(
+    [](auto&& arg) -> bool {
+      using T = std::decay_t<decltype(arg)>;
+      if constexpr (std::is_same_v<T, bool>) {  // NOLINT(bugprone-branch-clone)
+        return arg;
+      } else if constexpr (std::is_same_v<T, int>) {
+        return arg != 0;
+      } else if constexpr (std::is_same_v<T, double>) {
+        return arg != 0.0;
+      } else if constexpr (std::is_same_v<T, std::string>) {
+        return arg == "true";
+      } else {
+        static_assert(always_false_v<T>, "non-exhaustive visitor!");
+      }
+    },
+    m_value);
 }
 
 std::ostream& operator<<(std::ostream& os, const Variant& variant) {
-  VariantType variantType = variant.variantType();
+  const VariantType variantType = variant.variantType();
   if (variantType == VariantType::String) {
     os << variant.valueAsString();
   } else if (variantType == VariantType::Double) {
diff --git a/src/utilities/data/Variant.hpp b/src/utilities/data/Variant.hpp
index 4074035d097..9ef756f58af 100644
--- a/src/utilities/data/Variant.hpp
+++ b/src/utilities/data/Variant.hpp
@@ -14,6 +14,10 @@
 #include <optional>
 #include <variant>
 
+namespace Json {
+class Value;
+}
+
 namespace openstudio {
 
 // clang-format off
@@ -67,6 +71,12 @@ class UTILITIES_API Variant
   /// get value as string
   std::string valueAsString() const;
 
+  Json::Value valueAsJSON() const;
+
+  // No matter the underlying type, will check if it's true-ish
+  // Used for skipping a measure. eg true, "true", 1, 1.0 are all trueish
+  bool isTrueish() const;
+
  private:
   REGISTER_LOGGER("openstudio.Variant");
 
diff --git a/src/utilities/filetypes/WorkflowStepResult.cpp b/src/utilities/filetypes/WorkflowStepResult.cpp
index 51595a7615b..24b49011713 100644
--- a/src/utilities/filetypes/WorkflowStepResult.cpp
+++ b/src/utilities/filetypes/WorkflowStepResult.cpp
@@ -29,15 +29,8 @@ namespace detail {
       root["units"] = *m_units;
     }
 
-    if (m_value.variantType() == VariantType::String) {
-      root["value"] = m_value.valueAsString();
-    } else if (m_value.variantType() == VariantType::Double) {
-      root["value"] = m_value.valueAsDouble();
-    } else if (m_value.variantType() == VariantType::Integer) {
-      root["value"] = m_value.valueAsInteger();
-    } else if (m_value.variantType() == VariantType::Boolean) {
-      root["value"] = m_value.valueAsBoolean();
-    }
+    root["value"] = m_value.valueAsJSON();
+
     return root;
   }
 
@@ -90,6 +83,10 @@ namespace detail {
     return m_value.valueAsString();
   }
 
+  Json::Value WorkflowStepValue_Impl::valueAsJSON() const {
+    return m_value.valueAsJSON();
+  }
+
   void WorkflowStepValue_Impl::setName(const std::string& name) {
     m_name = name;
   }
@@ -400,7 +397,7 @@ namespace detail {
     for (const auto& stepValue : stepValues()) {
       boost::optional<Attribute> attribute;
 
-      VariantType variantType = stepValue.variantType();
+      const VariantType variantType = stepValue.variantType();
       switch (variantType.value()) {
         case VariantType::Boolean:
           attribute = Attribute(stepValue.name(), stepValue.valueAsBoolean());
@@ -655,11 +652,11 @@ WorkflowStepValue::WorkflowStepValue(const std::string& name, bool value)
 
 boost::optional<WorkflowStepValue> WorkflowStepValue::fromString(const std::string& s) {
   // We let it fail with a warning message
-  Json::CharReaderBuilder rbuilder;
+  const Json::CharReaderBuilder rbuilder;
   std::istringstream ss(s);
   std::string formattedErrors;
   Json::Value value;
-  bool parsingSuccessful = Json::parseFromStream(rbuilder, ss, &value, &formattedErrors);
+  const bool parsingSuccessful = Json::parseFromStream(rbuilder, ss, &value, &formattedErrors);
   if (!parsingSuccessful) {
     LOG(Warn, "Couldn't parse WorkflowStepValue from string s='" << s << "'. Error: '" << formattedErrors << "'.");
     return boost::none;
@@ -668,8 +665,8 @@ boost::optional<WorkflowStepValue> WorkflowStepValue::fromString(const std::stri
   boost::optional<WorkflowStepValue> result;
 
   try {
-    std::string name = value["name"].asString();
-    Json::Value v = value["value"];
+    const std::string name = value["name"].asString();
+    const Json::Value v = value["value"];
     if (v.isString()) {
       result = WorkflowStepValue(name, v.asString());
     } else if (v.isIntegral()) {
@@ -733,6 +730,10 @@ std::string WorkflowStepValue::valueAsString() const {
   return getImpl<detail::WorkflowStepValue_Impl>()->valueAsString();
 }
 
+Json::Value WorkflowStepValue::valueAsJSON() const {
+  return getImpl<detail::WorkflowStepValue_Impl>()->valueAsJSON();
+}
+
 void WorkflowStepValue::setName(const std::string& name) {
   getImpl<detail::WorkflowStepValue_Impl>()->setName(name);
 }
@@ -763,11 +764,11 @@ WorkflowStepResult::WorkflowStepResult(std::shared_ptr<detail::WorkflowStepResul
 
 boost::optional<WorkflowStepResult> WorkflowStepResult::fromString(const std::string& s) {
   // We let it fail with a warning message
-  Json::CharReaderBuilder rbuilder;
+  const Json::CharReaderBuilder rbuilder;
   std::istringstream ss(s);
   std::string formattedErrors;
   Json::Value value;
-  bool parsingSuccessful = Json::parseFromStream(rbuilder, ss, &value, &formattedErrors);
+  const bool parsingSuccessful = Json::parseFromStream(rbuilder, ss, &value, &formattedErrors);
   if (!parsingSuccessful) {
     LOG(Warn, "Couldn't parse WorkflowStepResult from string s='" << s << "'. Error: '" << formattedErrors << "'.");
     return boost::none;
@@ -792,7 +793,7 @@ boost::optional<WorkflowStepResult> WorkflowStepResult::fromString(const std::st
     }
 
     if (value.isMember("measure_type")) {
-      Json::Value measureType = value["measure_type"];
+      const Json::Value measureType = value["measure_type"];
       try {
         result.setMeasureType(MeasureType(measureType.asString()));
       } catch (const std::exception&) {
@@ -801,23 +802,23 @@ boost::optional<WorkflowStepResult> WorkflowStepResult::fromString(const std::st
     }
 
     if (value.isMember("measure_name")) {
-      Json::Value measureName = value["measure_name"];
+      const Json::Value measureName = value["measure_name"];
       result.setMeasureName(measureName.asString());
     }
 
     if (value.isMember("measure_uid")) {
-      Json::Value measureId = value["measure_uid"];
+      const Json::Value measureId = value["measure_uid"];
       result.setMeasureId(measureId.asString());
     }
 
     if (value.isMember("measure_version_id")) {
-      Json::Value versionId = value["measure_version_id"];
+      const Json::Value versionId = value["measure_version_id"];
       result.setMeasureVersionId(versionId.asString());
     }
 
     if (value.isMember("measure_version_modified")) {
-      Json::Value versionModified = value["measure_version_modified"];
-      std::string str = versionModified.asString();
+      const Json::Value versionModified = value["measure_version_modified"];
+      const std::string str = versionModified.asString();
       boost::optional<DateTime> dateTime = DateTime::fromISO8601(versionModified.asString());
       if (dateTime) {
         result.setMeasureVersionModified(dateTime.get());
@@ -825,27 +826,27 @@ boost::optional<WorkflowStepResult> WorkflowStepResult::fromString(const std::st
     }
 
     if (value.isMember("measure_xml_checksum")) {
-      Json::Value checksum = value["measure_xml_checksum"];
+      const Json::Value checksum = value["measure_xml_checksum"];
       result.setMeasureXmlChecksum(checksum.asString());
     }
 
     if (value.isMember("measure_class_name")) {
-      Json::Value className = value["measure_class_name"];
+      const Json::Value className = value["measure_class_name"];
       result.setMeasureClassName(className.asString());
     }
 
     if (value.isMember("measure_display_name")) {
-      Json::Value displayName = value["measure_display_name"];
+      const Json::Value displayName = value["measure_display_name"];
       result.setMeasureDisplayName(displayName.asString());
     }
 
     if (value.isMember("measure_taxonomy")) {
-      Json::Value taxonomy = value["measure_taxonomy"];
+      const Json::Value taxonomy = value["measure_taxonomy"];
       result.setMeasureTaxonomy(taxonomy.asString());
     }
 
     if (value.isMember("step_result")) {
-      StepResult stepResult(value["step_result"].asString());
+      const StepResult stepResult(value["step_result"].asString());
       result.setStepResult(stepResult);
     }
 
@@ -857,11 +858,10 @@ boost::optional<WorkflowStepResult> WorkflowStepResult::fromString(const std::st
       result.setStepFinalCondition(value["step_final_condition"].asString());
     }
 
-    Json::Value defaultArrayValue(Json::arrayValue);
-    Json::ArrayIndex n;
+    const Json::Value defaultArrayValue(Json::arrayValue);
 
     Json::Value errors = value.get("step_errors", defaultArrayValue);
-    n = errors.size();
+    Json::ArrayIndex n = errors.size();
     for (Json::ArrayIndex i = 0; i < n; ++i) {
       result.addStepError(errors[i].asString());
     }
@@ -886,7 +886,7 @@ boost::optional<WorkflowStepResult> WorkflowStepResult::fromString(const std::st
       // mimic the old StyledWriter behavior:
       wbuilder["indentation"] = "   ";
       for (Json::ArrayIndex i = 0; i < n; ++i) {
-        std::string stepval_str = Json::writeString(wbuilder, stepValues[i]);
+        const std::string stepval_str = Json::writeString(wbuilder, stepValues[i]);
         boost::optional<WorkflowStepValue> workflowStepValue = WorkflowStepValue::fromString(stepval_str);
         if (workflowStepValue) {
           result.addStepValue(*workflowStepValue);
diff --git a/src/utilities/filetypes/WorkflowStepResult.hpp b/src/utilities/filetypes/WorkflowStepResult.hpp
index b3eae761c90..5fcb0215b68 100644
--- a/src/utilities/filetypes/WorkflowStepResult.hpp
+++ b/src/utilities/filetypes/WorkflowStepResult.hpp
@@ -104,6 +104,8 @@ class UTILITIES_API WorkflowStepValue
   /// get value as string
   std::string valueAsString() const;
 
+  Json::Value valueAsJSON() const;
+
   //@}
   /** @name Setters */
   //@{
diff --git a/src/utilities/filetypes/WorkflowStepResult_Impl.hpp b/src/utilities/filetypes/WorkflowStepResult_Impl.hpp
index c73e5b5f7b5..0294f85863e 100644
--- a/src/utilities/filetypes/WorkflowStepResult_Impl.hpp
+++ b/src/utilities/filetypes/WorkflowStepResult_Impl.hpp
@@ -53,6 +53,8 @@ namespace detail {
 
     std::string valueAsString() const;
 
+    Json::Value valueAsJSON() const;
+
     //@}
     /** @name Setters */
     //@{
diff --git a/src/workflow/ApplyMeasure.cpp b/src/workflow/ApplyMeasure.cpp
index 0f8816857f5..d2d9a2e1220 100644
--- a/src/workflow/ApplyMeasure.cpp
+++ b/src/workflow/ApplyMeasure.cpp
@@ -39,19 +39,8 @@ bool isStepMarkedSkip(const std::map<std::string, openstudio::Variant>& stepArgs
 
   // handle skip first
   if (!stepArgs.empty() && stepArgs.contains("__SKIP__")) {
-    // TODO: handling of SKIP is incomplete here, will need to increment the runner and co, and not process the measure
     const openstudio::Variant& argumentValue = stepArgs.at("__SKIP__");
-    VariantType variantType = argumentValue.variantType();
-
-    if (variantType == VariantType::String) {
-      skip_measure = openstudio::ascii_to_lower_copy(argumentValue.valueAsString()) == "true";
-    } else if (variantType == VariantType::Double) {
-      skip_measure = (argumentValue.valueAsDouble() != 0.0);
-    } else if (variantType == VariantType::Integer) {
-      skip_measure = (argumentValue.valueAsInteger() != 0);
-    } else if (variantType == VariantType::Boolean) {
-      skip_measure = argumentValue.valueAsBoolean();
-    }
+    skip_measure = argumentValue.isTrueish();
   }
   return skip_measure;
 }
@@ -76,20 +65,30 @@ void OSWorkflow::applyMeasures(MeasureType measureType, bool energyplus_output_r
 
     auto stepArgs = step.arguments();
     const bool skip_measure = isStepMarkedSkip(stepArgs);
-    if (skip_measure || runner.halted()) {  // TODO: or halted
+    if (skip_measure || runner.halted()) {
       if (!energyplus_output_requests) {
         if (runner.halted()) {
           LOG(Info, fmt::format("Skipping measure '{}' because simulation halted", measureDirName));
         } else {
           LOG(Info, fmt::format("Skipping measure '{}'", measureDirName));
+          // required to update current step
+          runner.prepareForMeasureRun();
+
           WorkflowStepResult result = runner.result();
           runner.incrementStep();
-          // addResultMeasureInfo(result, bclMeasure);  // TODO: Should I really instantiate the BCLMeasure just for this?
+          const auto measureDirPath_ = workflowJSON.findMeasure(measureDirName);
+          if (measureDirPath_) {
+            BCLMeasure bclMeasure(measureDirPath_.get());
+            workflow::util::addResultMeasureInfo(result, bclMeasure);
+            const std::string className = bclMeasure.className();
+            const auto measureName = step.name().value_or(className);
+            output_attributes[measureName]["applicable"] = openstudio::Variant(false);
+          } else {
+            LOG(Warn, "Could not find measure '" << measureDirName << "', but it's marked as skipped anyways so ignoring.");
+            output_attributes[step.name().value_or(measureDirName)]["applicable"] = openstudio::Variant(false);
+          }
           result.setStepResult(StepResult::Skip);
         }
-
-        // Technically here I would need to have gotten className from the measure to match workflow-gem, just to set applicable = false
-        output_attributes[step.name().value_or(measureDirName)]["applicable"] = openstudio::Variant(false);
       }
       continue;
     }


Test patch:
diff --git a/src/measure/test/MeasureFixture.cpp b/src/measure/test/MeasureFixture.cpp
index 678b311f468..7c571896b65 100644
--- a/src/measure/test/MeasureFixture.cpp
+++ b/src/measure/test/MeasureFixture.cpp
@@ -88,9 +88,9 @@ void MeasureFixture::SetUpTestSuite() {
   openstudio::Logger::instance().standardOutLogger().disable();
 
   // clear scratch
-  openstudio::path p = resourcesPath() / openstudio::toPath("ruleset/scratch");
-  openstudio::filesystem::remove_all(p);
-  openstudio::filesystem::create_directories(p);
+  scratchDir = openstudio::getApplicationBuildDirectory() / openstudio::toPath("Testing/Temporary/measure");
+  openstudio::filesystem::remove_all(scratchDir);
+  openstudio::filesystem::create_directories(scratchDir);
 }
 
 void MeasureFixture::TearDownTestSuite() {
@@ -99,3 +99,4 @@ void MeasureFixture::TearDownTestSuite() {
 
 // static variables
 boost::optional<openstudio::FileLogSink> MeasureFixture::logFile;
+openstudio::path MeasureFixture::scratchDir;
diff --git a/src/measure/test/MeasureFixture.hpp b/src/measure/test/MeasureFixture.hpp
index 7a63aaa8fb9..355b04c8be3 100644
--- a/src/measure/test/MeasureFixture.hpp
+++ b/src/measure/test/MeasureFixture.hpp
@@ -39,6 +39,8 @@ class MeasureFixture : public ::testing::Test
   /// static data
   static boost::optional<openstudio::FileLogSink> logFile;
 
+  static openstudio::path scratchDir;
+
   virtual ~MeasureFixture() = default;
 };
 
diff --git a/src/measure/test/OSRunner_GTest.cpp b/src/measure/test/OSRunner_GTest.cpp
index daedb330282..0d9729def90 100644
--- a/src/measure/test/OSRunner_GTest.cpp
+++ b/src/measure/test/OSRunner_GTest.cpp
@@ -130,7 +130,7 @@ TEST_F(MeasureFixture, OSRunner_getArgumentValues) {
   argumentVector.push_back(requiredDoubleArgument);
 
   OSArgument requiredDoubleArgument2 = OSArgument::makeDoubleArgument("required_double2", true);
-  EXPECT_TRUE(requiredDoubleArgument2.setValue(234892384234.39485923845834534));
+  EXPECT_TRUE(requiredDoubleArgument2.setValue(1786.45));
   argumentVector.push_back(requiredDoubleArgument2);
 
   OSArgument optionalDoubleArgument = OSArgument::makeDoubleArgument("optional_double", false);
@@ -154,10 +154,7 @@ TEST_F(MeasureFixture, OSRunner_getArgumentValues) {
   OSArgument optionalStringArgument = OSArgument::makeStringArgument("optional_string", false);
   argumentVector.push_back(optionalStringArgument);
 
-  std::vector<std::string> choices;
-  choices.push_back("On");
-  choices.push_back("Off");
-
+  std::vector<std::string> choices{"On", "Off"};
   OSArgument requiredChoiceArgument = OSArgument::makeChoiceArgument("required_choice", choices, true);
   requiredChoiceArgument.setValue("Off");
   argumentVector.push_back(requiredChoiceArgument);
@@ -179,28 +176,32 @@ TEST_F(MeasureFixture, OSRunner_getArgumentValues) {
 
   std::map<std::string, OSArgument> argumentMap = convertOSArgumentVectorToMap(argumentVector);
 
-  bool b = runner.getBoolArgumentValue(requiredBoolArgument.name(), argumentMap);
+  // Create an extra argument that is just in the argumentMap and wouldn't be in the original measure argument vector
+  OSArgument extraArgument = OSArgument::makeStringArgument("an_extra_argument_not_expected", false);
+  argumentMap.emplace("an_extra_argument_not_expected", extraArgument);
+
+  const bool b = runner.getBoolArgumentValue(requiredBoolArgument.name(), argumentMap);
   EXPECT_TRUE(b);
 
-  bool b2 = runner.getBoolArgumentValue(requiredBoolArgument2.name(), argumentMap);
+  const bool b2 = runner.getBoolArgumentValue(requiredBoolArgument2.name(), argumentMap);
   EXPECT_FALSE(b2);
 
-  double d = runner.getDoubleArgumentValue(requiredDoubleArgument.name(), argumentMap);
+  const double d = runner.getDoubleArgumentValue(requiredDoubleArgument.name(), argumentMap);
   EXPECT_EQ(1.0, d);
 
-  double d2 = runner.getDoubleArgumentValue(requiredDoubleArgument2.name(), argumentMap);
-  EXPECT_EQ(234892384234.39485923845834534, d2);
+  const double d2 = runner.getDoubleArgumentValue(requiredDoubleArgument2.name(), argumentMap);
+  EXPECT_DOUBLE_EQ(1786.45, d2);
 
-  int i = runner.getIntegerArgumentValue(requiredIntegerArgument.name(), argumentMap);
+  const int i = runner.getIntegerArgumentValue(requiredIntegerArgument.name(), argumentMap);
   EXPECT_EQ(2, i);
 
-  std::string s = runner.getStringArgumentValue(requiredStringArgument.name(), argumentMap);
+  const std::string s = runner.getStringArgumentValue(requiredStringArgument.name(), argumentMap);
   EXPECT_EQ("Value", s);
 
-  std::string c = runner.getStringArgumentValue(requiredChoiceArgument.name(), argumentMap);
+  const std::string c = runner.getStringArgumentValue(requiredChoiceArgument.name(), argumentMap);
   EXPECT_EQ("Off", c);
 
-  boost::optional<openstudio::WorkspaceObject> w = runner.getOptionalWorkspaceObjectChoiceValue(optionalChoiceArgument2.name(), argumentMap, m);
+  const boost::optional<openstudio::WorkspaceObject> w = runner.getOptionalWorkspaceObjectChoiceValue(optionalChoiceArgument2.name(), argumentMap, m);
   EXPECT_TRUE(w);
 
   Json::Value argumentValues = runner.getArgumentValues(argumentVector, argumentMap);
@@ -212,7 +213,7 @@ TEST_F(MeasureFixture, OSRunner_getArgumentValues) {
   EXPECT_TRUE(argumentValues["optional_bool"].isNull());
 
   EXPECT_EQ(1.0, argumentValues["required_double"].asDouble());
-  EXPECT_EQ(234892384234.39485923845834534, argumentValues["required_double2"].asDouble());
+  EXPECT_DOUBLE_EQ(1786.45, argumentValues["required_double2"].asDouble());
   EXPECT_TRUE(argumentValues["optional_double"].isNull());
   EXPECT_FALSE(argumentValues["optional_double2"].isNull());
   EXPECT_EQ(10.5, argumentValues["optional_double2"].asDouble());
@@ -226,6 +227,8 @@ TEST_F(MeasureFixture, OSRunner_getArgumentValues) {
   EXPECT_EQ("Off", argumentValues["required_choice"].asString());
   EXPECT_TRUE(argumentValues["optional_choice"].isNull());
   EXPECT_EQ(openstudio::toString(boiler2.handle()), argumentValues["optional_choice2"].asString());
+
+  EXPECT_FALSE(argumentValues.isMember("an_extra_argument_not_expected"));
 }
 
 TEST_F(MeasureFixture, OSRunner_getPastStepValues) {
@@ -234,87 +237,151 @@ TEST_F(MeasureFixture, OSRunner_getPastStepValues) {
 
   std::vector<WorkflowStep> workflow_steps;
 
-  MeasureStep step1("Step1");
-  step1.setArgument("Argument1", 100);
-  step1.setArgument("Argument2", 200);
-  workflow_steps.push_back(step1);
-  WorkflowStepResult workflow_step_result1;
-  EXPECT_FALSE(workflow_step_result1.measureName());
-  workflow_step_result1.setMeasureName("MeasureName1");
-  ASSERT_TRUE(workflow_step_result1.measureName());
-  EXPECT_EQ("MeasureName1", workflow_step_result1.measureName().get());
-  WorkflowStepValue stepValue1("StepValue1", true);
-  workflow_step_result1.addStepValue(stepValue1);
-  WorkflowStepValue stepValue2("StepValue2", 5);
-  workflow_step_result1.addStepValue(stepValue2);
-
-  MeasureStep step2("Step2");
-  step2.setArgument("Argument3", 300);
-  step2.setArgument("Argument4", 400);
-  workflow_steps.push_back(step2);
-  WorkflowStepResult workflow_step_result2;
-  EXPECT_FALSE(workflow_step_result2.measureName());
-  workflow_step_result2.setMeasureName("MeasureName2");
-  ASSERT_TRUE(workflow_step_result2.measureName());
-  EXPECT_EQ("MeasureName2", workflow_step_result2.measureName().get());
-  WorkflowStepValue stepValue3("StepValue3", 342.3);
-  workflow_step_result2.addStepValue(stepValue3);
-  EXPECT_EQ(342.3, stepValue3.valueAsDouble());
-  WorkflowStepValue stepValue4("StepValue4", false);
-  workflow_step_result2.addStepValue(stepValue4);
-  EXPECT_FALSE(stepValue4.valueAsBoolean());
-  EXPECT_EQ(2, workflow_step_result2.stepValues().size());
+  {
+    MeasureStep step1("MeasureName1");
+    workflow_steps.push_back(step1);
+
+    step1.setArgument("Argument1", 100);
+    step1.setArgument("Argument2", 200);
+    EXPECT_EQ("MeasureName1", step1.measureDirName());
+    EXPECT_FALSE(step1.name());
+
+    {
+      WorkflowStepResult workflow_step_result1;
+      EXPECT_FALSE(workflow_step_result1.measureName());
+      workflow_step_result1.setMeasureName("measure_name_1");
+      ASSERT_TRUE(workflow_step_result1.measureName());
+      EXPECT_EQ("measure_name_1", workflow_step_result1.measureName().get());
+
+      const WorkflowStepValue stepValue1("StepValue1", true);
+      workflow_step_result1.addStepValue(stepValue1);
+
+      const WorkflowStepValue stepValue2("StepValue2", 5);
+      workflow_step_result1.addStepValue(stepValue2);
+
+      // Setting a Skip + (Completed At so it shows in the JSON)
+      workflow_step_result1.setStepResult(StepResult::Skip);
+      workflow_step_result1.setCompletedAt(DateTime::nowUTC());
+      step1.setResult(workflow_step_result1);
+    }
+  }
+
+  {
+    MeasureStep step2("MeasureName2");
+    workflow_steps.push_back(step2);
+
+    step2.setArgument("Argument3", 300);
+    step2.setArgument("Argument4", 400);
+
+    EXPECT_TRUE(step2.setName("My Step 2"));
+    EXPECT_EQ("MeasureName2", step2.measureDirName());
+    EXPECT_TRUE(step2.name());
+    EXPECT_EQ("My Step 2", step2.name().get());
+
+    {
+      WorkflowStepResult workflow_step_result2;
+      EXPECT_FALSE(workflow_step_result2.measureName());
+
+      workflow_step_result2.setMeasureName("measure_name_2");
+      ASSERT_TRUE(workflow_step_result2.measureName());
+      EXPECT_EQ("measure_name_2", workflow_step_result2.measureName().get());
+
+      const WorkflowStepValue stepValue3("StepValue3", 342.3);
+      workflow_step_result2.addStepValue(stepValue3);
+      EXPECT_EQ(342.3, stepValue3.valueAsDouble());
+
+      const WorkflowStepValue stepValue4("StepValue4", false);
+      workflow_step_result2.addStepValue(stepValue4);
+      EXPECT_FALSE(stepValue4.valueAsBoolean());
+
+      EXPECT_EQ(2, workflow_step_result2.stepValues().size());
+
+      // Step Result + (Completed At so it shows in the JSON)
+      workflow_step_result2.setStepResult(StepResult::Success);
+      workflow_step_result2.setCompletedAt(DateTime::nowUTC());
+      step2.setResult(workflow_step_result2);
+
+      {
+        ASSERT_TRUE(step2.result());
+        const WorkflowStepResult stepResult2_copy = step2.result().get();
+        ASSERT_TRUE(stepResult2_copy.stepResult());
+        EXPECT_EQ(workflow_step_result2.string(), stepResult2_copy.string());
+        ASSERT_TRUE(workflow_step_result2.stepResult());
+        EXPECT_EQ(StepResult{StepResult::Success}, workflow_step_result2.stepResult().get());
+      }
+    }
+  }
+
   EXPECT_TRUE(workflow.setWorkflowSteps(workflow_steps));
   EXPECT_EQ(2, workflow.workflowSteps().size());
   EXPECT_EQ(workflow.string(), runner.workflow().string());
-  workflow_step_result2.setStepResult(StepResult::Success);
-  step2.setResult(workflow_step_result2);
-  ASSERT_TRUE(step2.result());
-  WorkflowStepResult workflow_step_result2_ = step2.result().get();
-  ASSERT_TRUE(workflow_step_result2_.stepResult());
-  EXPECT_EQ(workflow_step_result2.string(), workflow_step_result2_.string());
-  ASSERT_TRUE(workflow_step_result2.stepResult());
-  EXPECT_EQ(StepResult::Success, workflow_step_result2.stepResult().get());
-
-  Json::Value stepValues;
-
-  stepValues = runner.getPastStepValuesForMeasure("MeasureName1");
-  EXPECT_EQ(0, stepValues.size());  // did not set step result
-
-  stepValues = runner.getPastStepValuesForMeasure("MeasureName2");
-  EXPECT_EQ(2, stepValues.size());
-
-  EXPECT_TRUE(stepValues["Argument3"].isNull());
-  EXPECT_TRUE(stepValues["Argument4"].isNull());
-  EXPECT_TRUE(stepValues["StepValue1"].isNull());
-  EXPECT_TRUE(stepValues["StepValue2"].isNull());
-  EXPECT_FALSE(stepValues["StepValue3"].isNull());
-  EXPECT_EQ(342.3, stepValues["StepValue3"].asDouble());
-  EXPECT_FALSE(stepValues["StepValue4"].isNull());
-  EXPECT_FALSE(stepValues["StepValue4"].asBool());
-
-  MeasureStep step3("Step3");
-  workflow_steps.push_back(step3);
-  WorkflowStepResult workflow_step_result3;
-  workflow_step_result3.setMeasureName("MeasureName3");
-  workflow_step_result3.setStepResult(StepResult::Success);
-  step3.setResult(workflow_step_result3);
-  WorkflowStepValue stepValue5("StepValue3", 20);
-  workflow_step_result3.addStepValue(stepValue5);
-  EXPECT_TRUE(workflow.setWorkflowSteps(workflow_steps));
-  EXPECT_EQ(3, workflow.workflowSteps().size());
 
-  stepValues = runner.getPastStepValuesForName("StepValue1");
-  EXPECT_EQ(0, stepValues.size());  // did not set step result
+  workflow.saveAs(scratchDir / "OSRunner_getPastStepValues_2steps.osw");
 
-  stepValues = runner.getPastStepValuesForName("StepValue3");
-  EXPECT_EQ(2, stepValues.size());
+  {
+    const Json::Value stepValues = runner.getPastStepValuesForMeasure("MeasureName1");
+    EXPECT_EQ(0, stepValues.size());  // did not set step result as Success
+  }
+
+  {
+    const Json::Value stepValues = runner.getPastStepValuesForMeasure("measure_name_1");
+    EXPECT_EQ(0, stepValues.size());  // did not set step result as Success
+  }
+
+  {
+    // possible keys:                      measureDirName, name       , StepResult's measureName
+    for (const std::string possible_key : {"MeasureName2", "My Step 2", "measure_name_2"}) {
+      Json::Value stepValues = runner.getPastStepValuesForMeasure(possible_key);
+      EXPECT_EQ(2, stepValues.size());
+
+      EXPECT_TRUE(stepValues["Argument3"].isNull());
+      EXPECT_TRUE(stepValues["Argument4"].isNull());
+      EXPECT_TRUE(stepValues["StepValue1"].isNull());
+      EXPECT_TRUE(stepValues["StepValue2"].isNull());
+      EXPECT_FALSE(stepValues["StepValue3"].isNull());
+      EXPECT_EQ(342.3, stepValues["StepValue3"].asDouble());
+      EXPECT_FALSE(stepValues["StepValue4"].isNull());
+      EXPECT_FALSE(stepValues["StepValue4"].asBool());
+    }
+  }
+
+  {
+    MeasureStep step3("MeasureName3");
+    workflow_steps.push_back(step3);
+    WorkflowStepResult workflow_step_result3;
+    workflow_step_result3.setMeasureName("measure_name_3");
+    workflow_step_result3.setStepResult(StepResult::Success);
+    workflow_step_result3.setCompletedAt(DateTime::nowUTC());
+
+    step3.setResult(workflow_step_result3);
+    const WorkflowStepValue stepValue5("StepValue3", 20);
+    workflow_step_result3.addStepValue(stepValue5);
+    EXPECT_TRUE(workflow.setWorkflowSteps(workflow_steps));
+    EXPECT_EQ(3, workflow.workflowSteps().size());
+  }
 
-  EXPECT_TRUE(stepValues["Step1"].isNull());
-  EXPECT_TRUE(stepValues["Step2"].isNull());
-  EXPECT_TRUE(stepValues["MeasureName1"].isNull());
-  EXPECT_FALSE(stepValues["MeasureName2"].isNull());
-  EXPECT_FALSE(stepValues["MeasureName3"].isNull());
-  EXPECT_EQ(342.3, stepValues["MeasureName2"].asDouble());
-  EXPECT_EQ(20, stepValues["MeasureName3"].asInt());
+  workflow.saveAs(scratchDir / "OSRunner_getPastStepValues_3steps.osw");
+
+  {
+    const Json::Value stepValues = runner.getPastStepValuesForName("StepValue1");
+    EXPECT_EQ(0, stepValues.size());  // did not set step result
+  }
+
+  {
+    Json::Value stepValues = runner.getPastStepValuesForName("StepValue3");
+    EXPECT_EQ(2, stepValues.size());
+
+    EXPECT_TRUE(stepValues["MeasureName1"].isNull());
+    EXPECT_TRUE(stepValues["measure_name_1"].isNull());
+
+    // We prefer the step name rather than measureDirName if available
+    EXPECT_TRUE(stepValues["MeasureName2"].isNull());
+    EXPECT_TRUE(stepValues["measure_name_2"].isNull());
+    EXPECT_FALSE(stepValues["My Step 2"].isNull());
+    EXPECT_EQ(342.3, stepValues["My Step 2"].asDouble());
+
+    EXPECT_TRUE(stepValues["measure_name_3"].isNull());
+    EXPECT_FALSE(stepValues["MeasureName3"].isNull());
+    EXPECT_EQ(20, stepValues["MeasureName3"].asInt());
+  }
 }