Fix treatment of distance for alternate backends #832
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Seems the tests are failing when attempting to generate a synthetic light curve with jktebop - will have to investigate further or fallback to just testing ellc for now. |
aprsa
approved these changes
Mar 28, 2024
Merged
kecnry
added a commit
that referenced
this pull request
Apr 1, 2024
* Import from within source tree overly restrictive (#819) In order to avoid same-name module conflicts with the system module (such as io.py), phoebe shouldn't be imported from its own source tree. The original test eliminated the entire package tree, including all auxiliary directories, which is overly restrictive and it prevented pytest from running when phoebe was installed with `pip -e`. This PR fixes that by blocking imports from the root directory and from the root/phoebe tree (the actual sources). Closes #806. * update tests for changes to pytest (#820) * Fixing up all pytests The new pytest version raises an error instead of warning when tests return a value, and when assert is used incorrectly. All tests have been touched up to not have issues with the latest pytest. * Update f-string to be compatible with python 3.7 f-string format f'{var=}' has been introduced in python 3.8, and it caused the 3.7 tests to fail. This fixes that issue. --------- Co-authored-by: Andrej Prsa <aprsa09@gmail.com> * Dynamical RVs now avoid meshing (#823) * Dynamical RVs now avoid meshing Calling b.compute_pblums() built the mesh and treated dynamical RVs as mesh-dependent. This fixes that for both run_compute() and for direct compute_pblums(), compute_l3(), and compute_ld_coeffs bundle methods. A new function, b._datasets_that_require_meshing(), filters datasets that require a mesh, i.e. 'lc', 'lp' and flux-weighted 'rv'. Closes #812. * Generalized dataset filtering with b._datasets_where() * Adding backend exception to mesh computation Even if phoebe backend didn't need meshes for a particular dataset, other backends do. This is now handled in the `_datasets_where()` function. --------- Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> * run_checks_compute() bugfix (#837) * run_checks_compute() bugfix The run_checks_compute() function included the internal "_default" compute parameter set in checking the ck2004 model atmosphere table existence in the passband files. This caused the checks to fail even if ck2004 was never needed. * Incorporating @kecnry's comments into the PR (along with additional comments) * Fixing the if statement per @kecnry's instructions. * Fix treatment of distance for alternate backends (#832) * regression test * don't duplicate distance handling for alt backends * temporarily skip jktebop * use top-level default_binary in test * version and changelog for 2.4.13 release * update setup-python to v5 to avoid deprecation warning --------- Co-authored-by: Andrej Prsa <aprsa@villanova.edu> Co-authored-by: Andrej Prsa <aprsa09@gmail.com>
aprsa
added a commit
that referenced
this pull request
Apr 1, 2024
* Import from within source tree overly restrictive (#819) In order to avoid same-name module conflicts with the system module (such as io.py), phoebe shouldn't be imported from its own source tree. The original test eliminated the entire package tree, including all auxiliary directories, which is overly restrictive and it prevented pytest from running when phoebe was installed with `pip -e`. This PR fixes that by blocking imports from the root directory and from the root/phoebe tree (the actual sources). Closes #806. * update tests for changes to pytest (#820) * Fixing up all pytests The new pytest version raises an error instead of warning when tests return a value, and when assert is used incorrectly. All tests have been touched up to not have issues with the latest pytest. * Update f-string to be compatible with python 3.7 f-string format f'{var=}' has been introduced in python 3.8, and it caused the 3.7 tests to fail. This fixes that issue. --------- Co-authored-by: Andrej Prsa <aprsa09@gmail.com> * Dynamical RVs now avoid meshing (#823) * Dynamical RVs now avoid meshing Calling b.compute_pblums() built the mesh and treated dynamical RVs as mesh-dependent. This fixes that for both run_compute() and for direct compute_pblums(), compute_l3(), and compute_ld_coeffs bundle methods. A new function, b._datasets_that_require_meshing(), filters datasets that require a mesh, i.e. 'lc', 'lp' and flux-weighted 'rv'. Closes #812. * Generalized dataset filtering with b._datasets_where() * Adding backend exception to mesh computation Even if phoebe backend didn't need meshes for a particular dataset, other backends do. This is now handled in the `_datasets_where()` function. --------- Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> * run_checks_compute() bugfix (#837) * run_checks_compute() bugfix The run_checks_compute() function included the internal "_default" compute parameter set in checking the ck2004 model atmosphere table existence in the passband files. This caused the checks to fail even if ck2004 was never needed. * Incorporating @kecnry's comments into the PR (along with additional comments) * Fixing the if statement per @kecnry's instructions. * Fix treatment of distance for alternate backends (#832) * regression test * don't duplicate distance handling for alt backends * temporarily skip jktebop * use top-level default_binary in test * version and changelog for 2.4.13 release * update setup-python to v5 to avoid deprecation warning --------- Co-authored-by: Andrej Prsa <aprsa@villanova.edu> Co-authored-by: Andrej Prsa <aprsa09@gmail.com>
aprsa
added a commit
that referenced
this pull request
Apr 1, 2024
* Import from within source tree overly restrictive (#819) In order to avoid same-name module conflicts with the system module (such as io.py), phoebe shouldn't be imported from its own source tree. The original test eliminated the entire package tree, including all auxiliary directories, which is overly restrictive and it prevented pytest from running when phoebe was installed with `pip -e`. This PR fixes that by blocking imports from the root directory and from the root/phoebe tree (the actual sources). Closes #806. * update tests for changes to pytest (#820) * Fixing up all pytests The new pytest version raises an error instead of warning when tests return a value, and when assert is used incorrectly. All tests have been touched up to not have issues with the latest pytest. * Update f-string to be compatible with python 3.7 f-string format f'{var=}' has been introduced in python 3.8, and it caused the 3.7 tests to fail. This fixes that issue. --------- Co-authored-by: Andrej Prsa <aprsa09@gmail.com> * Dynamical RVs now avoid meshing (#823) * Dynamical RVs now avoid meshing Calling b.compute_pblums() built the mesh and treated dynamical RVs as mesh-dependent. This fixes that for both run_compute() and for direct compute_pblums(), compute_l3(), and compute_ld_coeffs bundle methods. A new function, b._datasets_that_require_meshing(), filters datasets that require a mesh, i.e. 'lc', 'lp' and flux-weighted 'rv'. Closes #812. * Generalized dataset filtering with b._datasets_where() * Adding backend exception to mesh computation Even if phoebe backend didn't need meshes for a particular dataset, other backends do. This is now handled in the `_datasets_where()` function. --------- Co-authored-by: Kyle Conroy <kyleconroy@gmail.com> * run_checks_compute() bugfix (#837) * run_checks_compute() bugfix The run_checks_compute() function included the internal "_default" compute parameter set in checking the ck2004 model atmosphere table existence in the passband files. This caused the checks to fail even if ck2004 was never needed. * Incorporating @kecnry's comments into the PR (along with additional comments) * Fixing the if statement per @kecnry's instructions. * Fix treatment of distance for alternate backends (#832) * regression test * don't duplicate distance handling for alt backends * temporarily skip jktebop * use top-level default_binary in test * version and changelog for 2.4.13 release * update setup-python to v5 to avoid deprecation warning --------- Co-authored-by: Andrej Prsa <aprsa@villanova.edu> Co-authored-by: Andrej Prsa <aprsa09@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR avoids the duplicated handling of distance for alternate backends (except for legacy) that was erroneously introduced by #606.
This fixes #830