Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Make cutouts out of TICA cubes #60
Make cutouts out of TICA cubes #60
Changes from all commits
db9e269
f444a92
cc19b3d
a550525
63de19b
8bd3a3b
14164d1
5c2f4f9
e99582e
1a95cb2
d015b12
71b0fd5
8bf77fc
7910322
06439c8
e52d93c
f2d24cf
031037a
3a2c66b
309c43f
7aac689
408a8bc
73fa88b
0fae7b4
6a59345
01f0d42
3a3f041
66d96f1
458a35c
a68208c
050bc64
fea4919
f3b37e1
b3f95f0
c2351e5
a13e371
1287967
908bd16
1109fe0
6c64746
39fb556
675fb4e
85e3f63
8704ee4
e8a8ca6
7ed74cd
3b071be
cd64048
0074d0c
09ad86c
78564b2
a3957a4
270ed6f
25242a0
c9f8031
8eb05c8
ed9d420
bc4fa08
1431a64
cdc5520
50407ca
62c53d8
30b343f
7b9f163
1ae9a7d
1275009
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do use this function in tesscut. perhaps we shouldn't be, since it's "internal". or perhaps it should not be an internal function. nevertheless, it's fine as you have it, but it might have been nice to have had product be a keyword argument that defaulted to SPOC instead of being a positional arg so that downstream code wouldn't have to change. I believe you are also taking care of the downstream code, at least in tesscut. Are there any other packages that might use this function? Astroquery?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm fine with not changing this now. just pointing it out that we need to change it in tesscut (also in the s3_support branch, when we switch over to using this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in favor of changing this here to prevent any future bugs elsewhere. I'll work on this next.
EDIT: After second thought, it might be better to leave
product
as a required positional argument. We would still have to remember to change product to the Enum call wherever this function is on TESSCut, because we can't have SPOC as the default for both SPOC and TICA. And a "missing positional keyword argument" error is more straightforward to fix than whatever error would come out of attempting to parse SPOC info in a TICA table (it would probably be a missing keyword error, but still).And no
_parse_table_info
doesn't seem to be explicitly used in astroquery