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

Update documentation on I/O #267

Merged
merged 6 commits into from
Dec 20, 2024
Merged

Update documentation on I/O #267

merged 6 commits into from
Dec 20, 2024

Conversation

m-fila
Copy link
Contributor

@m-fila m-fila commented Dec 17, 2024

BEGINRELEASENOTES

  • Added documentation for reading and writing EDM4hep files with the IOSvc
  • Moved documentation on k4DataSvc to legacy page

ENDRELEASENOTES

Some extensions (namely sphinx-design and from myst_parser colon_fence) are used to render the pages more nicely in sphinx-based documentation with admonitions and tabs (setting properties either in steering file or from command line):
image

Closes #212


evtSvc = k4DataSvc("EventDataSvc")
io_svc = IOSvc("IOSvc")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if we need / want to mention this explicitly in the documentation but I think "IOSvc" is pretty much required as name for the IOSvc, right? At least some of the CLI examples below will only work with that. Can we create an empty IOSvc() and rely on a default name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a mention:

The service should be imported from k4FWCore and named "IOSvc" as other components may look for it under this name.

Yes, naming it differently might be a problem. Relaying on the default name works fine.
Added a comment

Copy link
Member

Choose a reason for hiding this comment

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

I checked to make sure and I didn't see any errors when using a different name or no name at all for IOSvc. Only the Reader and Writer retrieve it through a ServiceHandle.

README.md Outdated Show resolved Hide resolved
doc/LegacyPodioInputOutput.md Outdated Show resolved Hide resolved
doc/PodioInputOutput.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
doc/PodioInputOutput.md Outdated Show resolved Hide resolved
doc/PodioInputOutput.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@jmcarcell
Copy link
Member

I don't have any more comments. Anything else @m-fila?

@m-fila
Copy link
Contributor Author

m-fila commented Dec 20, 2024

I noticed that outputCommands is not capitalized as the rest of the IOSvc properties.
I'm not sure if you want to give it similar treatment as with Input and Output properties

Otherwise that's all. Thank you very much for the comments 😄

@jmcarcell
Copy link
Member

I had the same thought when I was reading the diffs. I'm not sure, in this case this was literally the same command as it is used with PodioDataSvc. At least with Output it was a different parameter name. So I don't have a strong opinion on changing it or keeping it as it is.

@jmcarcell jmcarcell enabled auto-merge (squash) December 20, 2024 11:17
@jmcarcell jmcarcell merged commit 5abd44d into key4hep:main Dec 20, 2024
6 of 7 checks passed
@m-fila m-fila deleted the update_docs branch December 20, 2024 11:29
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.

Update the documentation for reading / writing collections using IOSvc
3 participants