-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix: ReferenceMixin._ref weakrefs after deep copy #272
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #272 +/- ##
==========================================
+ Coverage 86.85% 86.95% +0.10%
==========================================
Files 25 25
Lines 1263 1273 +10
==========================================
+ Hits 1097 1107 +10
Misses 166 166 ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #272 will not alter performanceComparing Summary
|
@erickmartins, heads up: this PR is going to break omero-cli-transfer. specifically it looks like you were "locally fixing" this bug in omero-cli-transfer by calling unfortunately, using you can see how our test of omero-cli-transfer is now failing here: I'm afraid you'll have to either do some ome-types version specific logic now, use a try/catch, or check for the presence of that reference before trying to remove it. |
Appreciate the heads up! I'm about to go on vacation and might not have time to dig into this before then, but will do as soon as I'm back. |
ok, this is working now, but only for pydantic v2. I actually think that deepcopy with pydantic v1 may have had some subtle untested broken issues with already, that this is exposing. I'm skipping that test for now, and will probably just drop support for pydantic v1 altogether in another PR. it's been out for 1.5 years, plenty of time for libraries to update just enough to use the |
fixes #265