Skip to content

Bug was not there, but __repr__() now more robust in debug#242

Merged
lasofivec merged 3 commits intodevelfrom
Issue241_BuginRaysCompletedX12
Nov 5, 2019
Merged

Bug was not there, but __repr__() now more robust in debug#242
lasofivec merged 3 commits intodevelfrom
Issue241_BuginRaysCompletedX12

Conversation

@Didou09
Copy link
Copy Markdown
Member

@Didou09 Didou09 commented Nov 5, 2019

More informative camera construction from IDS

A warning has been added to camera construction from ids if some channels have nan in D or u

General Improvement

I noticed that the overloading of the parent class ToFuObjectBase.repr() method, though very useful when everything works fine, was a problem when in debug mode inside a class because the debugger wants to print the current variables, including self. But while debugging inside the class, self is not fully valid yet and the get_summary() medthod may not work, leading to the debugger bugging...

The parent method repr() is now more robust in debug mode: falls back to class.name if the current instance is not valid.

Issues

Issue #241 was not really an issue, closed

…() now more robust in debug mode: falls back to __class__.__name__ if instance not valid
@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Nov 5, 2019

Hello @Didou09! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-11-05 12:44:04 UTC

@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 5, 2019

Codecov Report

Merging #242 into devel will decrease coverage by 0.02%.
The diff coverage is 6.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #242      +/-   ##
==========================================
- Coverage   43.95%   43.93%   -0.03%     
==========================================
  Files          70       70              
  Lines       21278    21288      +10     
==========================================
  Hits         9352     9352              
- Misses      11926    11936      +10
Impacted Files Coverage Δ
tofu/imas2tofu/_core.py 0.74% <0%> (-0.01%) ⬇️
tofu/utils.py 48.9% <0%> (-0.07%) ⬇️
tofu/version.py 100% <100%> (ø) ⬆️

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 044d87f...eb7855a. Read the comment docs.

@lasofivec lasofivec merged commit fabc36a into devel Nov 5, 2019
@lasofivec lasofivec deleted the Issue241_BuginRaysCompletedX12 branch November 5, 2019 13:53
@lasofivec
Copy link
Copy Markdown
Collaborator

Erratum

The bug in Rays._complete_dX12() (see Issue #241) was not a bug: it was the input that was inappropriate (the line of sight geometry was not available in IDS).
=> No change to Rays class

I'm confused as to why mention that in this PR ?
If the issue wasn't an issue, comment there and close it ?
And thus this PR doesn't solve the Issue as it wasn't an issue...

@Didou09
Copy link
Copy Markdown
Member Author

Didou09 commented Nov 5, 2019

Agreed,
issue #241 commented and closed accordingly
This PR will simply serve as reference for it, but you are right, next time good practice would be to comment in issue and close it, since the PR doesn't change anything on this particular point

@lasofivec
Copy link
Copy Markdown
Collaborator

I changed your OP, but I think we could really juste erase the "erratum" paragraph.
Or copy and paste it to the issue, no?

@Didou09
Copy link
Copy Markdown
Member Author

Didou09 commented Nov 5, 2019

agreed

@Didou09
Copy link
Copy Markdown
Member Author

Didou09 commented Nov 5, 2019

done

@Didou09 Didou09 mentioned this pull request Nov 20, 2019
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.

4 participants