Skip to content
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

Replace tess-locator with tesswcs #10

Merged
merged 15 commits into from
Aug 14, 2024
Merged

Conversation

altuson
Copy link
Contributor

@altuson altuson commented Aug 6, 2024

  • Updated predict() function: uses tesswcs instead of tess-locator to determine the pixel location.
  • Created from_sector() function that allows you to initialize a TessEphem object with a target and sector.
  • In query to JPL Horizons, return astrometric RA, Dec instead of apparent RA, Dec.
  • Updated and added examples to README file.
  • Added tests to test_ephem.py for new functions.

@altuson altuson changed the title WIP Update to tesswcs WIP: Replace tess-locator with tesswcs Aug 6, 2024
@altuson
Copy link
Contributor Author

altuson commented Aug 7, 2024

I've run pytests, black and flake8 locally so these should pass.

@altuson altuson changed the title WIP: Replace tess-locator with tesswcs Replace tess-locator with tesswcs Aug 7, 2024
@jorgemarpa
Copy link
Collaborator

GH actions are failing because these were defined with python = 3.8 and now the TOML file asks for 3.9.
Is 3.9 necessary because of something specific or can we keep 3.8?

pyproject.toml Show resolved Hide resolved
src/tess_ephem/ephem.py Show resolved Hide resolved
src/tess_ephem/ephem.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jorgemarpa jorgemarpa left a comment

Choose a reason for hiding this comment

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

I've updated the actions in the main brunch. Now you have to pull from upstream in your local branch (tess-wcs) to get the updates. Then push to your branch to update the PR.
That should fix the errors on mypy and black8. You have to run black again before pushing to fix one file formatting.
Also, now it will run pytest when making a new PR.

@altuson
Copy link
Contributor Author

altuson commented Aug 13, 2024

I've updated the actions in the main brunch. Now you have to pull from upstream in your local branch (tess-wcs) to get the updates. Then push to your branch to update the PR. That should fix the errors on mypy and black8. You have to run black again before pushing to fix one file formatting. Also, now it will run pytest when making a new PR.

@jorgemarpa I have merged your changes into my tess-wcs branch and run black locally to update the file. It should be ready for you to run the workflows and merge with the main branch.

@altuson altuson requested a review from jorgemarpa August 13, 2024 21:22
@jorgemarpa
Copy link
Collaborator

@altuson: the actions did not pass because Poetry changed how to install developer dependencies, it works as a group now. In the pyproject.toml file, you updated to the new way to group dependencies, this is great! But it made the workflows crash as they were not installing the devs dependencies.
I fix this in the action files and it should run fine now.

Now you should pull from the upstream/main branch these changes to update your branch and the PR.

Sorry we are running into all these small issues. These are because we haven't updated this repo in a while, so many things have changed a bit from last time.

This would be the last one 🤞🏾

@altuson
Copy link
Contributor Author

altuson commented Aug 14, 2024

@altuson: the actions did not pass because Poetry changed how to install developer dependencies, it works as a group now. In the pyproject.toml file, you updated to the new way to group dependencies, this is great! But it made the workflows crash as they were not installing the devs dependencies. I fix this in the action files and it should run fine now.

Now you should pull from the upstream/main branch these changes to update your branch and the PR.

Sorry we are running into all these small issues. These are because we haven't updated this repo in a while, so many things have changed a bit from last time.

This would be the last one 🤞🏾

@jorgemarpa I've merged the new changes into this branch. Fingers crossed it all runs smoothly now!

@jorgemarpa
Copy link
Collaborator

jorgemarpa commented Aug 14, 2024

@altuson we are very close now. The action errors are due to different python syntax between 3.9 and 3.10 for the type hints of a function that accepts two types of variables for an argument. I suggest using Optional from typing to avoid this error. See my recommended code line in the files.

src/tess_ephem/ephem.py Outdated Show resolved Hide resolved
@altuson
Copy link
Contributor Author

altuson commented Aug 14, 2024

@altuson we are very close now. The action errors are due to different python syntax between 3.9 and 3.10 for the type hints of a function that accepts two types of variables for an argument. I suggest using Optional from typing to avoid this error. See my recommended code line in the files.

@jorgemarpa I have made your suggested update.

@jorgemarpa
Copy link
Collaborator

LGTM!
merging...

@jorgemarpa jorgemarpa merged commit 0c5fdf2 into SSDataLab:main Aug 14, 2024
5 checks passed
@altuson altuson deleted the tess-wcs branch August 14, 2024 19:29
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.

2 participants