search

byte-physics / igortest

Igor Pro Universal Testing Framework
https://docs.byte-physics.de/igor-unit-testing-framework/
BSD 3-Clause "New" or "Revised" License
7 stars 2 forks source link

Function Tags #383

Closed Garados007 closed 1 year ago

Garados007 commented 1 year ago

This PR contains the following changes:

Close #378 Close #328

Garados007 commented 1 year ago

@t-b @MichaelHuth Ready for Review.

t-b commented 1 year ago

Review:

f632d98c (Refactor: Move function tags functions to own file, 2023-01-16)

Looks good.

fc2f76ac (Function Tags: Fix long function name bug, 2023-01-16)

The approach here with the quick and long path seems quite complicated. Why not just use a 1D textwave for storing the full function names?

e3c99406 (Function Tags: Cleanup function tags, 2023-01-16)

I think CleanupName could replace SanitizeName or?

The representation of the function tag itself is kept to keep compability with older experiments.

We don't need that. In case it would make your live easier.

Tests are missing for both fixes.

t-b commented 1 year ago

Another comment:

In GetFunctionTagWave the line

tag_constants[][1] = SanitizeTagName(tag_constants[p][0])

will result in the cleaned up name being found. But I don't think this is what we want.

Garados007 commented 1 year ago

@t-b

The approach here with the quick and long path seems quite complicated. Why not just use a 1D textwave for storing the full function names?

Do you want to throw dimension labels away at all and use FindValue in one wave (text wave) to find the index which is at the same time the same index in the second wave (wave reference wave)? My fix tried to keep the old behavior as much as possible and used the part with the dimension labels to its limits.

We don't need that. In case it would your live easier.

That's good. It is easier to change the tag to a clean one in the first place than the part with the cleanup. So I will change this completely.

t-b commented 1 year ago

Do you want to throw dimension labels away at all and use FindValue in one wave (text wave) to find the index which is at the same time the same index in the second wave (wave reference wave)? My fix tried to keep the old behavior as much as possible and used the part with the dimension labels to its limits.

Yes that would be my favourite.

Garados007 commented 1 year ago

@t-b @MichaelHuth Ready for next review round.