wbalmer / backtracks

Python package to fit relative astrometry with background star motion tracks.
https://backtracks.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
6 stars 2 forks source link

Fixed issue with corr_terms and confidence envelopes, and more #23

Closed tomasstolker closed 9 months ago

tomasstolker commented 9 months ago

Hi @wbalmer and @gotten,

This PR contains a bit more changes then I initially planned to include so please have a look if you agree with these changes 😉.

There are two issue that I fixed:

A couple of changes to the parameters:

And some more updates:

Great that the package is now available on PyPI! I noticed that the name has been changed to backtracks. I have changed the name also in the folder and filenames. @wbalmer probably best to also change the name of the Github repo?

About the package structure: I would suggest to simplify it to just having a backtracks folder (like before) instead of embedded in src. I think that is a quite common package structure, in particular because there are only a few files. But let me know in case you disagree then I will change it back!

Also, I would recommend to not add any binary files, so only include text files. Any changes to a binary file will store a full copy of the file so quickly increasing the repo size. I just checked and it is 70 MB already when cloning from Github. I have therefore removed all binary files that seemed not needed to include. The unit tests run in a few minutes so I think it is fine to run the actual fit instead of including a precalculated fit. If you want to run them, just use pytest in the main folder.

Finally, I tested the installation with Python 3.11 (with pip install .) and that seems to work fine as well!

tomasstolker commented 9 months ago

Hereby an example of the trackplot for HD 131399:

HD_131399A_model_bgstar

wbalmer commented 9 months ago

This is really excellent, thank you so much for clarifying the package structure, the pytests, etc. I'll review in just a few minutes and merge.

tomasstolker commented 9 months ago

Sorry for the many commits... It took me a while to figure out how to setup the Actions workflow because installing orbitize caused an error. You can squash the commits if you decide to merge. But let me know if you have any comments!