Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide acq_name and acq_source to append method when filtering #999

Conversation

Jamezo97
Copy link
Contributor

The channel group acquisition properties of a filtered MDF aren't copied across for MDF versions 4.20 and greater. See the example below for a minimal reproducible example:

from asammdf import MDF, Signal

t, v = [0, 1], [2, 4]
acq_name = "ExampleTask"
signal_name = f"Task.Struct.Value"

for version in ("4.10", "4.20"):
    mdf1 = MDF(version=version)
    mdf1.append([Signal(v, t, name=signal_name)], acq_name=acq_name)

    mdf2 = mdf1.filter([signal_name])
    cg_nr, _ = mdf2.channels_db[signal_name][0]
    
    result_acq_name = mdf2.groups[cg_nr].channel_group.acq_name
    print(f"Version {version}: {result_acq_name}")

I would expect this to print

Version 4.10: ExampleTask
Version 4.20: ExampleTask

However instead we see

Version 4.10: ExampleTask
Version 4.20: None

This appears to be due to the call to _append_column_oriented for 4.20 version MDFs, which creates multiple groups. However the filter method only updates the acquisition properties for the first group.

A simple fix is to pass the acq_source and acq_name to the append method. However I am unsure if this is the proper solution. Perhaps the current behavior is intended? Or perhaps this change has further ramifications?

@danielhrisca danielhrisca merged commit f980d98 into danielhrisca:development Apr 17, 2024
18 checks passed
@Jamezo97 Jamezo97 deleted the bugfix/copy-acquisition-properties-when-filtering branch April 18, 2024 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants