AllenInstitute / MIES

Multichannel Igor Electrophysiology Suite
https://alleninstitute.github.io/MIES/user.html
Other
21 stars 6 forks source link

SF: Parser/Executor improvements #2133

Closed MichaelHuth closed 3 weeks ago

MichaelHuth commented 4 weeks ago
MichaelHuth commented 4 weeks ago

@t-b There seems to be some issue in the CI where the ipf converter for the docu fails on a function declaration. My guess is that it stumbles upon wave type flags in the multi return declaration.

t-b commented 4 weeks ago

@t-b There seems to be some issue in the CI where the ipf converter for the docu fails on a function declaration. My guess is that it stumbles upon wave type flags in the multi return declaration.

Better bug reports next time please.

From CI:

sphinx-build says: 
/home/ci/Packages/doc/file/_m_i_e_s___sweep_formula_8ipf.rst:6: WARNING: Error when parsing function declaration.
If the function has no return type:
  Error in declarator or parameters-and-qualifiers
  Invalid C++ declaration: Expecting "(" in parameters-and-qualifiers. [error at 36]
    static std::tuple< WAVE, WaveText > SF_ExecutorCreateOrCheckNumeric (WaveDouble/z out, WaveTextOrNull outT, variable size0, variable size1, variable size2, variable size3)
    ------------------------------------^
If the function has a return type:
  Error in declarator or parameters-and-qualifiers
  If pointer to member declarator:
    Invalid C++ declaration: Expected '::' in pointer to member (function). [error at 68]
      static std::tuple< WAVE, WaveText > SF_ExecutorCreateOrCheckNumeric (WaveDouble/z out, WaveTextOrNull outT, variable size0, variable size1, variable size2, variable size3)
      --------------------------------------------------------------------^
  If declarator-id:
    Invalid C++ declaration: Expecting "," or ")" in parameters-and-qualifiers, got "/". [error at 79]
      static std::tuple< WAVE, WaveText > SF_ExecutorCreateOrCheckNumeric (WaveDouble/z out, WaveTextOrNull outT, variable size0, variable size1, variable size2, variable size3)
      -------------------------------------------------------------------------------^

I've fixed that in https://github.com/byte-physics/doxygen-filter-ipf/pull/27.

t-b commented 3 weeks ago

Review:

3698bf62e (SF: Add support for operation results in arrays, 2024-05-27)

Looks mostly good.

I'm wondering though what

+    WaveStats/Q/M=1 convert
+    if(V_npnts)
+        return $""
+    endif

is supposed to do. This looks like a check for at least one finite value, and in that case HasOneValidEntry should be preferred. Or a new helper function or flag for HasOneValidEntry.

I've also created https://github.com/AllenInstitute/MIES/issues/2138 so that we get a function which returns the wave dimensionality correctly.

104514e7a (Update doxygen-filter-ipf submodule to latest main, 2024-06-06)

Looks good. For the future no need to cite the submodule commit hash in the message. This is part of the diff.

commit 104514e7a71e43d8a4682299b64e6a872727e45a
Author: Michael Huth <Michael.Huth@byte-physics.de>
Date:   Thu Jun 6 12:38:18 2024 +0200

    Update doxygen-filter-ipf submodule to latest main

    commit 9f442b45bb6d2580eb0e51a0567a9d9a897f37d3

    This fixes wave flags in MR syntax in MR function declarations

diff --git a/Packages/doc/doxygen-filter-ipf b/Packages/doc/doxygen-filter-ipf
index d229d451f..9f442b45b 160000
--- a/Packages/doc/doxygen-filter-ipf
+++ b/Packages/doc/doxygen-filter-ipf
@@ -1 +1 @@
-Subproject commit d229d451f35e88b8c38edb8c26533acfbc3fd10f
+Subproject commit 9f442b45bb6d2580eb0e51a0567a9d9a897f37d3

ca30a596f (SF: Fix parser error for +, -, , / and other operations inside arrays/operation, 2024-05-29) 857cf0b74 (SF: Add data type forwarding for primitive operations +, -, , /, 2024-05-23)

Looks both good.

t-b commented 3 weeks ago

CI failure here:

  Entering test case "UTF_SweepFormula#CheckMixingNonFiniteAndText"

  0: is false. Assertion "FAIL()" failed in UTF_SweepFormula#CheckMixingNonFiniteAndText (UTF_SweepFormula.ipf, line 4428)

  Leaving test case "UTF_SweepFormula#CheckMixingNonFiniteAndText"