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

Allowing the users to append their filters into the existing BEAST filter library #756

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

galaxyumi
Copy link
Contributor

This PR is to allow the users to add any filters that are not in the default BEAST filter library into the filter library from command line. An input ascii file for a filter throughput curve should have two columns: wavelength in angstrom and throughput.

@codecov-commenter
Copy link

Codecov Report

Merging #756 (eae674a) into master (f206c52) will decrease coverage by 0.05%.
The diff coverage is 12.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #756      +/-   ##
==========================================
- Coverage   43.31%   43.26%   -0.06%     
==========================================
  Files         101      101              
  Lines        9811     9825      +14     
==========================================
+ Hits         4250     4251       +1     
- Misses       5561     5574      +13     
Impacted Files Coverage Δ
beast/observationmodel/phot.py 36.48% <12.50%> (-1.50%) ⬇️
beast/tools/subgridding_tools.py 32.59% <0.00%> (-0.38%) ⬇️

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 f206c52...eae674a. Read the comment docs.

Comment on lines +736 to +776


if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument(
"filter_throughput_curve",
type=str,
default=None,
help="filename of filter throughput curve to be added into the BEAST filter library",
)
parser.add_argument(
"--tablename",
type=str,
help="Table name for the filter",
)
parser.add_argument(
"--observatory",
type=str,
help="Observatory for the filter (e.g., HST, GALEX)",
)
parser.add_argument(
"--instrument",
type=str,
help="Instrument associated with the filter (WFC3, ACS, ...)",
)
parser.add_argument(
"--filtername",
type=str,
help="Name for the filter",
)

args = parser.parse_args()

if args.filter_throughput_curve:
lamb, throughput = numpy.loadtxt(args.filter_throughput_curve, usecols=(0, 1),
unpack=True)
append_filter(lamb, throughput, args.tablename, args.observatory, args.instrument,
args.filtername)

if not any(vars(args).values()):
parser.print_help()
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be a separate script in the tools subdir instead of added to phot.py. For consistency with how similar capabilities are handled in the BEAST.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. Will start a separate script for this functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This task could be merged with the outstanding WIP PR #646!

Copy link
Member

Choose a reason for hiding this comment

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

Need to add this script to this PR.

docs/append_filters.rst Show resolved Hide resolved
Copy link
Member

@karllark karllark left a comment

Choose a reason for hiding this comment

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

Need to move the script from phot.py to an appropriately named file in the tools directory.

@karllark
Copy link
Member

This PR supersedes PR #646, right? If so, please confirm that it provides the same capabilities or that those are not needed desired anymore.

@karllark
Copy link
Member

Ping.

@karllark
Copy link
Member

Ping. ;-)

@galaxyumi
Copy link
Contributor Author

Ping. ;-)

Sorry!! Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants