Skip to content

TPC: IDC task update - visualize most recent data#1640

Merged
Barthelemy merged 1 commit into
AliceO2Group:masterfrom
tklemenz:idcNew
Mar 29, 2023
Merged

TPC: IDC task update - visualize most recent data#1640
Barthelemy merged 1 commit into
AliceO2Group:masterfrom
tklemenz:idcNew

Conversation

@tklemenz
Copy link
Copy Markdown
Contributor

This adds the possibility for the IDC task to fetch the latest data from the ccdb regardless of the validity range and given timestamp. The functionality was moved from the temperature task to the Utility class. Therefore, the temperature task was also adjusted.

In addition, the option to exclude IDCDelta entirely from being visualized via the config file was added. The option is doIDCDelta.

@tklemenz tklemenz requested a review from wiechula as a code owner February 14, 2023 13:32
@tklemenz
Copy link
Copy Markdown
Contributor Author

@wiechula When you have no objections, I remove the WIP.

@tklemenz tklemenz changed the title [WIP] TPC: IDC task update - visualize most recent data TPC: IDC task update - visualize most recent data Feb 22, 2023
@tklemenz tklemenz mentioned this pull request Feb 23, 2023
@tklemenz
Copy link
Copy Markdown
Contributor Author

The errors are unrelated. @wiechula, if everything is fine, this can be merged.

@Barthelemy
Copy link
Copy Markdown
Collaborator

@wiechula Could you approve and merge ? thank you

@tklemenz
Copy link
Copy Markdown
Contributor Author

tklemenz commented Mar 9, 2023

Hi @Barthelemy, sorry for not changing the title to [WIP]. We discussed and there are a few changes that I want to make. I'll do them asap and remove the WIP again.

@tklemenz tklemenz changed the title TPC: IDC task update - visualize most recent data [WIP] TPC: IDC task update - visualize most recent data Mar 9, 2023
@tklemenz
Copy link
Copy Markdown
Contributor Author

tklemenz commented Mar 9, 2023

@wiechula, I made all the requested changes. Especially the splitString function is gone and replaced by tokenize. I needed to make a few more changes to make it work with the tokenize function.

@tklemenz tklemenz changed the title [WIP] TPC: IDC task update - visualize most recent data TPC: IDC task update - visualize most recent data Mar 16, 2023
Copy link
Copy Markdown
Collaborator

@wiechula wiechula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for me

Comment thread Modules/TPC/include/TPC/Utility.h Outdated
/// Splits strings at delimiter and puts the tokens into a vector
/// \param inString String to be split
/// \param delimiter Delimiter where string should be split
std::vector<std::string> splitString(const std::string inString, const char* delimiter);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tklemenz , did you see this comment? I think it was not resolved, yet. If I understand correctly, you added

#include "CommonUtils/StringUtils.h"

I the cxx, but didn't replace splitString usage by https://github.com/AliceO2Group/AliceO2/blob/dev/Common/Utils/include/CommonUtils/StringUtils.h#L110 and removed splitString.

Copy link
Copy Markdown
Contributor Author

@tklemenz tklemenz Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is tokenize in Utility.cxx line 174, which was splitString before. That should be it and works without problems.

@tklemenz
Copy link
Copy Markdown
Contributor Author

Hi @Barthelemy, this can be merged now.

@Barthelemy Barthelemy merged commit b8fd8aa into AliceO2Group:master Mar 29, 2023
knopers8 pushed a commit to knopers8/QualityControl that referenced this pull request Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants