-
Notifications
You must be signed in to change notification settings - Fork 23
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
Set input_units, return_units, input_units_equivalencies, and bounding_box to use astropy's built-in input preparation? #237
Comments
I would definitely entertain such a PR. One question, would this allow for no units to be passed and interpreted as wavenumbers (1/micron)? Not allowing this is not a show-shopper, I've thought about removing this option. It would potentially mean new versions are not compatible with existing code, but that can be ok/allowed. |
There is the |
Define the `input_units`, `return_units`, `input_units_equivalencies`, and `bounding_box` properties for all models. Use Astropy models' built-in unit conversion support. All models now require inputs with valid units (wavelength, wavenumber, or frequency). Dimensionless inputs are no longer automatically converted to wavenumber. Fixes karllark#237.
Define the `input_units`, `return_units`, `input_units_equivalencies`, and `bounding_box` properties for all models. Use Astropy models' built-in unit conversion support. All models now require inputs with valid units (wavelength, wavenumber, or frequency). Dimensionless inputs are no longer automatically converted to wavenumber. Fixes karllark#237.
If you set the
input_units
,return_units
,input_units_equivalencies
, andbounding_box
properties, then when you call a model, theastropy.modeling.Model
base class will automatically convert the users' arguments to your desired units and do bounds checking. Note that thespectral()
equivalency will do automatic conversion between wavelength and wavenumber. This builtin functionality could essentially replace_get_x_in_wavenumbers
and_test_valid_x_range
. Would you entertain a patch to do this? I'm working on a project where it would be nice to have the bounds and units of these models exposed in the conventional way.The text was updated successfully, but these errors were encountered: