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

IOSvc: Minor fixes for improving usage and errors #213

Merged
merged 18 commits into from
Sep 5, 2024
Merged

Conversation

jmcarcell
Copy link
Contributor

@jmcarcell jmcarcell commented Jul 18, 2024

BEGINRELEASENOTES

  • Fail if IOSvc or ApplicationMgr are not imported with from k4FWCore import IOSvc (it can be imported from Configurables). The python wrapper in k4FWCore does a few minor things and importing from Configurables doesn't fail currently and can lead to hard to solve issues.
  • Add an error message when reading a file without the events category. Currently it fails with an error in python that is not very clear.
  • Use Input and Output for IOSvc, deprecate input and output since all the other properties are capitalized. The same for IOType instead of ioType. This will print a warning about a duplicated property that should be ignored for now.
  • Suppress two warnings about using external input when using the default EventLoopMgr.
  • Import ApplicationMgr always from k4FWCore

ENDRELEASENOTES

The two warnings are these:

EventLoopMgr      WARNING Unable to locate service "EventSelector"
EventLoopMgr      WARNING No events will be processed from external input.

they will appear always and this feature is not being used. In any case, this only overrides the default case, someone can create an EventLoopMgr and have the warnings. I opened an issue in Gaudi.

The most important is failing when importing ApplicationMgr from Configurables as most of the examples do not set the algorithms for reading and writing that the user doesn't need to worry about in almost any case, but the wrapper in k4FWCore does so one only needs to specify IOSvc.Input or IOSvc.Output.

@jmcarcell
Copy link
Contributor Author

This should be ready to be merged. I will merge today if there aren't any comments.

@andresailer andresailer self-requested a review August 9, 2024 10:54
python/k4FWCore/IOSvc.py Show resolved Hide resolved
python/k4FWCore/IOSvc.py Show resolved Hide resolved
k4FWCore/python/k4FWCore/utils.py Outdated Show resolved Hide resolved
k4FWCore/python/k4FWCore/utils.py Show resolved Hide resolved
python/k4FWCore/ApplicationMgr.py Show resolved Hide resolved
python/k4FWCore/ApplicationMgr.py Show resolved Hide resolved
Copy link
Contributor

@m-fila m-fila left a comment

Choose a reason for hiding this comment

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

This should be changed to .Input and .Output too, no?

iosvc = IOSvc("IOSvc")
iosvc.input = "functional_producer_multiple.root"
iosvc.output = "functional_mix_iosvc.root"

Copy link
Contributor

@m-fila m-fila left a comment

Choose a reason for hiding this comment

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

If think it might be simpler if we say to just always import ApplicationMgr from k4FWCore rather than if there is IOSvc then import from k4FWCore otherwise import from k4FWCore or Configurables.

In a similar vain maybe all the examples could import from k4FWCore rather than importing from Configurables when possible?

Maybe in a longer run we could port our custom python wrappers (ApplicationMgr.py, IOSvc.py) back to C++ (no extra checks or python parsing needed on our side, users can import all they need from Configurables, easier to extend by users if needed)

k4FWCore/python/k4FWCore/utils.py Show resolved Hide resolved
k4FWCore/components/IOSvc.h Show resolved Hide resolved
@andresailer andresailer changed the title Minor fixes for improving usage and errors IOSvc: Minor fixes for improving usage and errors Aug 20, 2024
@jmcarcell
Copy link
Contributor Author

If think it might be simpler if we say to just always import ApplicationMgr from k4FWCore rather than if there is IOSvc then import from k4FWCore otherwise import from k4FWCore or Configurables.

In a similar vain maybe all the examples could import from k4FWCore rather than importing from Configurables when possible?

I agree this is a good idea and now all the examples import from k4FWCore.

Maybe in a longer run we could port our custom python wrappers (ApplicationMgr.py, IOSvc.py) back to C++ (no extra checks or python parsing needed on our side, users can import all they need from Configurables, easier to extend by users if needed)

About this I'm not sure, for ApplicationMgr that would require a reimplementation of the Gaudi one in C++ and then it can't be called ApplicationMgr anymore since there will always be already one available...

@jmcarcell jmcarcell merged commit e0778b3 into main Sep 5, 2024
4 of 9 checks passed
@jmcarcell jmcarcell deleted the minor-fixes branch September 5, 2024 10:35
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.

3 participants