spacetelescope / ullyses

Code to create products for the ULLYSES program
https://ullyses.stsci.edu/
BSD 3-Clause "New" or "Revised" License
2 stars 6 forks source link

Get wrapper and coadd working, add test #14

Closed stscirij closed 2 years ago

jotaylor commented 2 years ago

I installed the package, went to the tests/ directory and ran pytest. I got quite a few errors though, which I've attached as a txt file here. Any advice Robert? I'm a bad coder and don't use pytest often so need some help 😅 pytest.txt

stscirij commented 2 years ago

Hi Jo,

It's hard to tell from this - do you have ullyses_utils installed? Looks like a problem getting the target coordinates file, maybe check that when you import ullyses_utils that ullyses_utils.path points where you would expect it to. I would also make sure you clean up the tests directory before trying again - the test script cleans up after itself if everything works, but if there's an error it can leave some data directories lying around...

And you are most definitely NOT a bad coder - the quality of your code is WAY higher than most of the code from scientists (who aren't primarily developers) that I've seen...

Robert J.


From: Jo Taylor @.***> Sent: Tuesday, March 29, 2022 4:23:21 PM To: spacetelescope/ullyses Cc: Robert Jedrzejewski; Author Subject: Re: [spacetelescope/ullyses] Get wrapper and coadd working, add test (PR #14)

I installed the package, went to the tests/ directory and ran pytest. I got quite a few errors though, which I've attached as a txt file here. Any advice Robert? I'm a bad coder and don't use pytest often so need some help 😅 pytest.txthttps://urldefense.com/v3/__https://github.com/spacetelescope/ullyses/files/8374765/pytest.txt__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!ycnwC4N62tnlbiXCdn28-tJ11uurEeWzyKBvQgFT59OGszt-x5J5qzd3PAiKCNfQD6rDnaYgoxqbP3i5UDPhiQ$

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/spacetelescope/ullyses/pull/14*issuecomment-1082338388__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!ycnwC4N62tnlbiXCdn28-tJ11uurEeWzyKBvQgFT59OGszt-x5J5qzd3PAiKCNfQD6rDnaYgoxqbP3hPLOQVnQ$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AEYCW6U6HDRYKCOF6W4BOG3VCNRDTANCNFSM5R7BNACQ__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!ycnwC4N62tnlbiXCdn28-tJ11uurEeWzyKBvQgFT59OGszt-x5J5qzd3PAiKCNfQD6rDnaYgoxqbP3iapae7HA$. You are receiving this because you authored the thread.Message ID: @.***>

stscirij commented 2 years ago

if you do

import ullyses_utils import os os.listdir(ullyses_utils.path[0] + "/data/target_metadata")

you should get a directory listing that includes the pd_targetinfo.json and pd_all_aliases.json files somewhere in the list.

Robert J.


From: Jo Taylor @.***> Sent: Tuesday, March 29, 2022 4:23:21 PM To: spacetelescope/ullyses Cc: Robert Jedrzejewski; Author Subject: Re: [spacetelescope/ullyses] Get wrapper and coadd working, add test (PR #14)

I installed the package, went to the tests/ directory and ran pytest. I got quite a few errors though, which I've attached as a txt file here. Any advice Robert? I'm a bad coder and don't use pytest often so need some help 😅 pytest.txthttps://urldefense.com/v3/__https://github.com/spacetelescope/ullyses/files/8374765/pytest.txt__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!ycnwC4N62tnlbiXCdn28-tJ11uurEeWzyKBvQgFT59OGszt-x5J5qzd3PAiKCNfQD6rDnaYgoxqbP3i5UDPhiQ$

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/spacetelescope/ullyses/pull/14*issuecomment-1082338388__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!ycnwC4N62tnlbiXCdn28-tJ11uurEeWzyKBvQgFT59OGszt-x5J5qzd3PAiKCNfQD6rDnaYgoxqbP3hPLOQVnQ$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AEYCW6U6HDRYKCOF6W4BOG3VCNRDTANCNFSM5R7BNACQ__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!ycnwC4N62tnlbiXCdn28-tJ11uurEeWzyKBvQgFT59OGszt-x5J5qzd3PAiKCNfQD6rDnaYgoxqbP3iapae7HA$. You are receiving this because you authored the thread.Message ID: @.***>

jotaylor commented 2 years ago

@stscirij I think I know what the problem is? The coadd code is looking for the target info file, pd_targetinfo.json, which didn't get ported to the ullyses-utils repo. (It's in the website repo instead. Don't ask 😩 ) I'm making a branch in the utils repo to fix this now.

jotaylor commented 2 years ago

Also, sorry, should have said that I did confirm I have ullyses-utils installed, and could see the necessary files. I also tried deleting the output from the previous pytest run, which didn't help. Hopefully adding the targetinfo file fix things.

jotaylor commented 2 years ago

Okay, after PR#17 was merged in ullyses-utils (https://github.com/spacetelescope/ullyses-utils/pull/17), it looks like the wrapper ran correctly. However, there were some errors in the testing code in the cleanup. Is it just a permissions issue @stscirij ? I've attached the log here. pytest.log

stscirij commented 2 years ago

Hi Jo,

Looks like you were working in a directory that is a symbolic link. When I run the tests, I am in the tests directory of my checked out repo on my mac, and it runs without problems (apart from a few warnings that I'll hopefully get to one day...).

If you need to be able to run from a directory that is a symbolic link, I could put in some code to test for that and work around it.

Also, I'm, working on getting the ctts_cal script running - sorry I wasn't at the DP meeting yesterday, I was at a farewell lunch for one of our group members and it ran on much longer than expected...

Robert J.


From: Jo Taylor @.***> Sent: Tuesday, April 5, 2022 12:08:29 PM To: spacetelescope/ullyses Cc: Robert Jedrzejewski; Mention Subject: Re: [spacetelescope/ullyses] Get wrapper and coadd working, add test (PR #14)

Okay, after PR#17 was merged in ullyses-utils (spacetelescope/ullyses-utils#17https://urldefense.com/v3/__https://github.com/spacetelescope/ullyses-utils/pull/17__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!zCIDVygCsFjIoIcLArArtyxYkrqpVXWLzDSiAyK6g-DMsVOn9AUuG0ye3vSpzEoFEw11-MvTcfu78pl2jGkF_g$), it looks like the wrapper ran correctly. However, there were some errors in the testing code in the cleanup. Is it just a permissions issue @stscirijhttps://urldefense.com/v3/__https://github.com/stscirij__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!zCIDVygCsFjIoIcLArArtyxYkrqpVXWLzDSiAyK6g-DMsVOn9AUuG0ye3vSpzEoFEw11-MvTcfu78pkjmTIRaw$ ? I've attached the log here. pytest.loghttps://urldefense.com/v3/__https://github.com/spacetelescope/ullyses/files/8419952/pytest.log__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!zCIDVygCsFjIoIcLArArtyxYkrqpVXWLzDSiAyK6g-DMsVOn9AUuG0ye3vSpzEoFEw11-MvTcfu78pkNkfQ34w$

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/spacetelescope/ullyses/pull/14*issuecomment-1088939653__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!zCIDVygCsFjIoIcLArArtyxYkrqpVXWLzDSiAyK6g-DMsVOn9AUuG0ye3vSpzEoFEw11-MvTcfu78pkOlYeJtg$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AEYCW6UPQY46SO42CLLWO63VDRQP3ANCNFSM5R7BNACQ__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!zCIDVygCsFjIoIcLArArtyxYkrqpVXWLzDSiAyK6g-DMsVOn9AUuG0ye3vSpzEoFEw11-MvTcfu78pkmVDU8Kw$. You are receiving this because you were mentioned.Message ID: @.***>

jotaylor commented 2 years ago

I'm working in the tests directory of the cloned repo as well 🤔 @emsnyder does it work for you? You need to uninstall and reinstall ullyses_utils before trying the test again

jotaylor commented 2 years ago

@stscirij FYI I already made some significant updates to ctts_cal.py so make sure you have the latest main branch here before you do any further edits.

stscirij commented 2 years ago

@jotaylor I have your changes from 23 days ago - was there anything since then?

jotaylor commented 2 years ago

@jotaylor I have your changes from 23 days ago - was there anything since then?

nope, that's it! good luck testing...

jotaylor commented 2 years ago

The issue was determined to be running pytest on central store. It's not clear why this creates symlinks but when the repo is cloned into a local directory pytest works fine.