F2I-Consulting / fesapi

API for ENERGISTICS™ data standards (mainly RESQML™), multi-languages (C++, Java, C#, Python)
Apache License 2.0
34 stars 24 forks source link

pushBackFloatHdf5Array1dOfValues with no min max supplied does not compute them #262

Closed kmgoss closed 3 years ago

kmgoss commented 3 years ago

What are the steps to reproduce this issue?

  1. Call ContinuousProperty::pushBackFloatHdf5Array1dOfValues() with no min and no max value supplied

What does happen?

No min max values are computed and written

What were you expecting to happen?

Min and max values should be computed as per the comments above the method declaration

Any logs, error output, etc?

n/a

Any other comments?

The default values of NaN are used, which causes the code to call pushBackFloatHdf5ArrayOfValues(float const values, unsigned long long const numValues, unsigned int numArrayDimensions, COMMON_NS::AbstractHdfProxy proxy = nullptr, float minimumValue = nullptr, float * maximumValue = nullptr) with the default value of nullptr for the last 2 parameters. The implementation of this method does not call setPropertyMinMax() when the mins and maxes are nullptrs (ContinuousProperty.cpp line 292).

What versions of fesapi are you using?

v1.2.1.0

philippeVerney commented 3 years ago

Hi @kmgoss,

Thanks for your bug report! Have you given a look to coming v2.0 which is current master branch? I think that I have refactored properties min max computation in it?

Or do you need to stick to a v1.* and request a v1.2.2 for example? FYI, v2.0 should be released this month.

kmgoss commented 3 years ago

We will be sticking with v1. for the next couple of years I believe I am working out the mins and maxes in my code and passing them through to fesapi. This works - so there is no need for an update to v1. in this regard. I raised the bug 1) for completeness and 2) in case you are still actively working on v1.2. Regards, Keith

philippeVerney commented 3 years ago

Thanks a lot for your answer and your help to make FESAPI better.

I'll keep this bug open and when time permits, I will:

If this issue becomes critical for you or others, please let me know and I will raise the priority of this task according to my available time.

philippeVerney commented 3 years ago

Thanks @kmgoss for having reported this bug. This helps a lot the coming release.