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

various (small) improvements #291

Merged
merged 7 commits into from
Dec 4, 2023
Merged

Conversation

marcomusy
Copy link
Contributor

Description

What is this PR

  • camera.py and cameras.py: rename old 'clippingRange' -> 'clipping_range' rename old 'focalPoint' -> 'focal_point'
  • render.py: remove import matplotlib.pyplot as plt, this looks like a genuine bug
  • correct close() method accordingly
  • add actor._mesh.transform = None in if isinstance(actor._mesh, VedoVolume) as future vedo release will need this
  • settings.py: set WHOLE_SCREEN = False as default, avoid setting size='full' as this is poorly dealt with in vtk
  • fix output path in examples/screenshot.py and video.py
  • pyproject.toml: a newer version of "morphapi>=0.2" seems to be needed

How has this PR been tested?

Tested on the examples

Is this a breaking change?

No

Does this PR require an update to the documentation?

No

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes

marcomusy and others added 3 commits December 1, 2023 18:48
  avoid importing the full vtk namespace (speed)
  rename old 'clippingRange' -> 'clipping_range'
  rename old 'focalPoint' -> 'focal_point'
- render.py
  remove import "matplotlib.pyplot as plt", this looks like a genuine bug and
  correct close() method
  avoid setting size='full' as this is poorly dealt with in vtk
- add "actor._mesh.transform = None" in "if isinstance(actor._mesh, VedoVolume)"
  as future vedo release will need this
- settings.py:
  set WHOLE_SCREEN = False
- fix output path in examples/screenshot.py and video.py
- pyproject.toml:
  a newer version of "morphapi>=0.2" seems to be needed
Copy link
Member

@alessandrofelder alessandrofelder left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this @marcomusy

I made some tweaks to the tests to adapt to your changes and made other tiny suggestions.
Still some trouble with test_volumetric_data but I will keep investigating.

pyproject.toml Outdated Show resolved Hide resolved
brainrender/actors/volume.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 4, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (19350ce) 86.18% compared to head (97e184e) 86.07%.

Files Patch % Lines
brainrender/camera.py 88.88% 1 Missing ⚠️
brainrender/render.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #291      +/-   ##
==========================================
- Coverage   86.18%   86.07%   -0.11%     
==========================================
  Files          26       26              
  Lines        1209     1214       +5     
==========================================
+ Hits         1042     1045       +3     
- Misses        167      169       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alessandrofelder
Copy link
Member

Opened #294 for test that still doesn't pass - may well be the data we regress against in that test.

@alessandrofelder alessandrofelder merged commit 2f54304 into brainglobe:main Dec 4, 2023
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants