Skip to content

Add copy() method for DiffractionObject #204

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

Merged
merged 6 commits into from
Dec 8, 2024
Merged

Conversation

bobleesj
Copy link
Contributor

@bobleesj bobleesj commented Dec 7, 2024

Closes #201 - add copy() for DiffracitonObject

xtype="tth",
)
copy_of_DO = do.copy()
assert do == copy_of_DO
Copy link
Contributor Author

@bobleesj bobleesj Dec 7, 2024

Choose a reason for hiding this comment

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

Utilzing the object's __eq__ nice implementation

Copy link
Contributor

Choose a reason for hiding this comment

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

To complete this test also assert id(actual) != Id(initial)

Copy link
Contributor

Choose a reason for hiding this comment

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

Then the copy is not pointing to the same object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes done below.

@bobleesj
Copy link
Contributor Author

bobleesj commented Dec 7, 2024

@sbillinge ready for review

Copy link

codecov bot commented Dec 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.54%. Comparing base (08de7c9) to head (ca0ef30).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #204      +/-   ##
==========================================
+ Coverage   96.49%   96.54%   +0.05%     
==========================================
  Files           8        8              
  Lines         342      347       +5     
==========================================
+ Hits          330      335       +5     
  Misses         12       12              
Files with missing lines Coverage Δ
tests/test_diffraction_objects.py 88.42% <100.00%> (+0.64%) ⬆️

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

Pls see inline

xtype="tth",
)
copy_of_DO = do.copy()
assert do == copy_of_DO
Copy link
Contributor

Choose a reason for hiding this comment

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

To complete this test also assert id(actual) != Id(initial)

xtype="tth",
)
copy_of_DO = do.copy()
assert do == copy_of_DO
Copy link
Contributor

Choose a reason for hiding this comment

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

Then the copy is not pointing to the same object

@bobleesj
Copy link
Contributor Author

bobleesj commented Dec 7, 2024

@sbillinge Right! Thank you. Additional test added.

@bobleesj
Copy link
Contributor Author

bobleesj commented Dec 7, 2024

@sbillinge ready for review - just enhanced the variable name from copy_of_DO to do_copy for consistency.

@sbillinge
Copy link
Contributor

@bobleesj not sure why codecov is complaining here...diffpy has a global org secret token. Maybe it is being flaky. I will just try and rerun it? after than an empty commit?

@bobleesj
Copy link
Contributor Author

bobleesj commented Dec 8, 2024

Screenshot 2024-12-08 at 10 05 26 AM

Codecov looks okay to me in fact 0.05% has gone up! @sbillinge

@sbillinge
Copy link
Contributor

Screenshot 2024-12-08 at 10 05 26 AM

Codecov looks okay to me in fact 0.05% has gone up! @sbillinge

it failed to upload so failed. I reran it and it uploaded and passed..... CC is officially flaky....

@sbillinge sbillinge merged commit 77b884c into diffpy:main Dec 8, 2024
5 checks passed
@bobleesj bobleesj deleted the copy-obj branch December 8, 2024 16:36
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.

add a copy method to diffraction objects
2 participants