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

update syntax to remove unnecessary underscores after poppy PR 386 #395

Merged

Conversation

mperrin
Copy link
Collaborator

@mperrin mperrin commented Jan 6, 2021

Update to remove now-unnecessary underscores before the [_]get_optical_system function.

Needed to fix failing tests after spacetelescope/poppy#386

@mperrin mperrin self-assigned this Jan 6, 2021
@mperrin mperrin added the bug Something isn't working label Jan 6, 2021
@codecov
Copy link

codecov bot commented Jan 6, 2021

Codecov Report

Merging #395 (9d0eabc) into develop (dec015b) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #395   +/-   ##
========================================
  Coverage    48.29%   48.29%           
========================================
  Files           14       14           
  Lines         5721     5721           
========================================
  Hits          2763     2763           
  Misses        2958     2958           
Impacted Files Coverage Δ
webbpsf/wfirst.py 89.77% <ø> (ø)
webbpsf/webbpsf_core.py 78.61% <100.00%> (ø)

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 dec015b...9d0eabc. Read the comment docs.

Copy link
Contributor

@shanosborne shanosborne left a comment

Choose a reason for hiding this comment

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

It looks like most the the tests have passed. However, the py37-stable test (an allowed failure) did fail though in the same way that the other test envs in 391 are failing

@mperrin
Copy link
Collaborator Author

mperrin commented Jan 6, 2021

However, the py37-stable test (an allowed failure) did fail though in the same way that the other test envs in 391 are failing

That makes sense: the stable test cases uses the released stable poppy, so it's inconsistent with the fix here for dev poppy. That test case is an allowed failure precisely for cases like this.

@mperrin
Copy link
Collaborator Author

mperrin commented Jan 6, 2021

@shanosborne Are you able & ready to approve this so we can merge? Given that the one allowed failure case in CI is indeed allowed.

@shanosborne shanosborne self-requested a review January 6, 2021 18:53
@mperrin mperrin merged commit aaca6cf into spacetelescope:develop Jan 6, 2021
@mperrin mperrin deleted the update_syntax_after_poppy386 branch November 6, 2021 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants