Skip to content

Conversation

@jmcarcell
Copy link
Member

@jmcarcell jmcarcell commented Jul 29, 2025

An issue in python (wlav/cppyy#312) where a RecursionError is thrown seems to happen because we have an operator-> for collections. Regardless of that, I think it doesn't fit with the rest of podio and should be removed. I have made a few PRs not to use this:

BEGINRELEASENOTES

  • Deprecate the generation of an operator-> for collections which made them usable as if they were pointers even when dealing with regular values.
    • Depending on your usage, the removal of this operator might break code that uses a generated datamodel. The recommended fix is to simply switch to using . instead of -> when working with (non-pointer) collections.
    • The original purpose for adding this was the (now abandoned) seamless drop-in replacement for LCIO, which uses collection pointers instead of values.

ENDRELEASENOTES

@m-fila
Copy link
Contributor

m-fila commented Jul 29, 2025

Yes! Some time ago we discussed we don't want it in common collection interface and it's mostly for historical reasons

Copy link
Collaborator

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Curious that this is only in place for the collections. I thought we also had this for the handles, but I couldn't find it.

@tmadlener tmadlener merged commit 76e88cb into AIDASoft:master Aug 11, 2025
22 checks passed
@jmcarcell
Copy link
Member Author

The deprecation seems to generate a lot of warnings in python scripts. Reproducer:

import podio
import edm4hep
frame = podio.Frame()
particles = frame.put(edm4hep.MCParticleCollection(), "MCParticleCollection")

and the warnings:

input_line_76:7:90: warning: 'operator->' is deprecated: Use of operator->() is deprecated. Use . instead. [-Wdeprecated-declarations]
      new (ret) (edm4hep::MCParticleCollection*) (((edm4hep::MCParticleCollection*)obj)->operator->());
                                                                                         ^
libedm4hepDict dictionary payload:2494:5: note: 'operator->' has been explicitly marked deprecated here
  [[deprecated("Use of operator->() is deprecated. Use . instead.")]]
    ^
input_line_76:11:53: warning: 'operator->' is deprecated: Use of operator->() is deprecated. Use . instead. [-Wdeprecated-declarations]
      (void)(((edm4hep::MCParticleCollection*)obj)->operator->());
                                                    ^
libedm4hepDict dictionary payload:2494:5: note: 'operator->' has been explicitly marked deprecated here
  [[deprecated("Use of operator->() is deprecated. Use . instead.")]]

@tmadlener
Copy link
Collaborator

Does this script still work with #812 in place? If yes, then we could make a tag and merge that one after that.

@tmadlener
Copy link
Collaborator

I can't reproduce this with my local stack, where the only obvious major difference is the python version. I can however reproduce this on the nightlies. Building #812 and then EDM4hep on top of the nightlies keeps the script working without the warnings.

AFAIU this the PRs mentioned at the top comment are all "backwards" compatible, i.e. they will keep working with a new podio tag and will work on older podio tags as well. The previous versions however might stop working with #812 merged.

@tmadlener
Copy link
Collaborator

@wdconinc @nathanwbrei I don't think you will have any issues with it, but just as a heads up to look out for some potential deprecation warnings when switching to podio@1.4

tmadlener pushed a commit to tmadlener/podio that referenced this pull request Aug 14, 2025
…ers (AIDASoft#811)

* Deprecate operator-> for objects that are not pointers

* Fix the deprecation warning in podio

---------

Co-authored-by: jmcarcell <jmcarcell@users.noreply.github.com>
jmcarcell added a commit to jmcarcell/acts that referenced this pull request Nov 4, 2025
-> was deprecated in AIDASoft/podio#811 (v1.4 of Podio)
and removed in AIDASoft/podio#812 (v1.5 of Podio). Note
that . always works for any version.
tmadlener added a commit to tmadlener/JANA2 that referenced this pull request Nov 4, 2025
This is necessary to build with podio >= 1.5, but will work for all
older versions of podio regardless.

Deprecated in AIDASoft/podio#811
Removed in AIDASoft/podio#812
kodiakhq bot pushed a commit to acts-project/acts that referenced this pull request Nov 4, 2025
-> was deprecated in AIDASoft/podio#811 (v1.4 of Podio)
and removed in AIDASoft/podio#812 (v1.5 of Podio). Note
that . always works for any version.
njacazio pushed a commit to AliceO2Group/acts that referenced this pull request Nov 8, 2025
-> was deprecated in AIDASoft/podio#811 (v1.4 of Podio)
and removed in AIDASoft/podio#812 (v1.5 of Podio). Note
that . always works for any version.
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