Skip to content
This repository has been archived by the owner on Oct 14, 2023. It is now read-only.

Add __reduce__ to Body class #1443

Merged
merged 3 commits into from
Jan 16, 2022
Merged

Conversation

Yash-10
Copy link
Member

@Yash-10 Yash-10 commented Jan 16, 2022

This change does seem to solve the problem mentioned in #1395. __reduce__ returns a string, which I think is commonly done for singleton classes. As a result, I was able to get the plot:
fig

Just wondering if this is really a solution to the issue...

@codecov
Copy link

codecov bot commented Jan 16, 2022

Codecov Report

Merging #1443 (9ca7563) into main (93f9fab) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1443   +/-   ##
=======================================
  Coverage   91.83%   91.84%           
=======================================
  Files          95       95           
  Lines        4448     4450    +2     
  Branches      430      430           
=======================================
+ Hits         4085     4087    +2     
  Misses        273      273           
  Partials       90       90           
Impacted Files Coverage Δ
src/poliastro/bodies.py 85.71% <100.00%> (+0.52%) ⬆️

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 93f9fab...9ca7563. Read the comment docs.

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Looks like __reduce__ accepts either a string, and that it's the right thing to do in this case:

If a string is returned, the string should be interpreted as the name of a global variable. It should be the object’s local name relative to its module; the pickle module searches the module namespace to determine the object’s module. This behaviour is typically useful for singletons.

https://docs.python.org/3/library/pickle.html#object.__reduce__

So, this solution is good! But it needs a test 😄

@Yash-10
Copy link
Member Author

Yash-10 commented Jan 16, 2022

I used the tmp_path fixture (https://docs.pytest.org/en/latest/how-to/tmp_path.html#the-tmp-path-fixture) to save and load objects into a file for testing. Had not used it before though. Hopefully it's not an overkill 😅

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Looks perfect!

@astrojuanlu astrojuanlu merged commit 069167a into poliastro:main Jan 16, 2022
@Yash-10 Yash-10 deleted the pickle-body branch January 16, 2022 20:05
@astrojuanlu astrojuanlu mentioned this pull request Feb 10, 2022
19 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants