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

Remove the copy() method and make __copy__ an error. #400

Merged
merged 1 commit into from Dec 14, 2018

Conversation

Projects
3 participants
@ssteinbach
Copy link
Member

ssteinbach commented Dec 13, 2018

Again, anticipating the C++ implementation, we are removing the shallow copy function. It doesn't look like anyone in the code base was using it.

@ssteinbach ssteinbach added this to the Public Beta 10 milestone Dec 13, 2018

@ssteinbach ssteinbach requested review from jminor , reinecke and davidbaraff Dec 13, 2018

@ssteinbach ssteinbach added this to To Do in C++ API via automation Dec 13, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 13, 2018

Codecov Report

Merging #400 into master will decrease coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #400      +/-   ##
==========================================
- Coverage   87.05%   86.99%   -0.07%     
==========================================
  Files          63       63              
  Lines        5569     5565       -4     
==========================================
- Hits         4848     4841       -7     
- Misses        721      724       +3
Impacted Files Coverage Δ
opentimelineio_contrib/adapters/hls_playlist.py 90.99% <ø> (ø) ⬆️
opentimelineio/core/serializable_object.py 90.74% <100%> (-0.64%) ⬇️
...ontrib/adapters/tests/test_hls_playlist_adapter.py 99.53% <100%> (ø) ⬆️
opentimelineio/core/composition.py 92.82% <0%> (-1.54%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb60a6f...05c5540. Read the comment docs.

@jminor

jminor approved these changes Dec 14, 2018

@@ -893,11 +893,7 @@ def test_range_nested(self):
]
)

# Subtle point, but copy() of a Track returns a new Track with a copy
# of all of its metadata, but not of its children. To get that you
# need to deepcopy().

This comment has been minimized.

@jminor

jminor Dec 14, 2018

Collaborator

Yeah, that was pretty odd. The new behavior is clear and simple.

@jminor jminor merged commit bbf248a into PixarAnimationStudios:master Dec 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

C++ API automation moved this from To Do to Done Dec 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment