danielhrisca / asammdf

Fast Python reader and editor for ASAM MDF / MF4 (Measurement Data Format) files
GNU Lesser General Public License v3.0
631 stars 223 forks source link

get_can_signal return values varies if Signal contains only text convers values #504

Open tobiasandorfer opened 3 years ago

tobiasandorfer commented 3 years ago

Python version

('python=3.8.7 (tags/v3.8.7:6503f05, Dec 21 2020, 17:59:51) [MSC v.1928 64 bit ' '(AMD64)]') os=Windows-10-10.0.17763-SP0' numpy=1.20.0 asammdf=6.2.0

Description

When a signal has a value that stays at a text conversion value over the compleat measurment and the "ignore_value2text_conversion" Parameter is False the return values are "text" values (Test 1) In the "Test_1_1" file i change between different "text conversion values, behavior is the same as in "Test 1" scenario

In case there is a mix of text conversion and non text conversion values the return values that have value description are returned as nan values (Test 2)

image

I recognised the behavior on a multiplexed signal, but found out, that it´s the same for non mux signals

since the returned "text" values crash numpy calculations i was hoping the behavior could be fixed in a way that the scenario in "Test 1" would return non values

thanks again for the amazing work best regards Tobias Value_2_Text_issue.zip

danielhrisca commented 3 years ago

Hello Tobias,

this is the intended behavior right now since a numpy array cannot contain mixed dtypes.

the returned "text" values crash numpy calculations

how does this happen on your side?

tobiasandorfer commented 3 years ago

Hello Daniel,

thanks for the feedback! The way i used the "value2textconversion" argument was to get the physically measurment values only (this is my commen usecase) so when text values get returend as "nan values" this works perfekt for me since there are a lot of numpy functions like nanmax or nanmin that just dont use the "nan values" for calculations.

I now understand that the "value2textconversion" is not designed for my usecase. Do you think it would be possible to bring a "physically_vals_only" argument into the function? I'd be happy to make a commit with my idea.

danielhrisca commented 11 months ago

@tobiasandorfer how is release 7.3.16 working?

tobiasandorfer commented 9 months ago

It took me some time to reconstruct the test and understand the initial problem from my point of view it looks like the behaviour has not changed I produced the same output as in the table in the first post version was 7.3.18