GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Refactor imwidget by apasarkar · Pull Request #459 · fastplotlib/fastplotlib

Conversation

@apasarkar

@apasarkar apasarkar commented Mar 29, 2024 •
edited by kushalkolar
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Closes #284

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

EricThomson reacted with rocket emoji
All reactions
  • 🚀 1 reaction
apasarkar added 3 commits March 28, 2024 19:25
@apasarkar
Removes slider_dims and now always generates slides for the first n -…
3662ab6
… 2 dimensions
@apasarkar
Adds more checks, removing C from the default dims order
a95badc
@apasarkar
Removes some more code that allowed specifying dimensions by int and …
d155c27
…by character also updates old documentation
@apasarkar
apasarkar requested review from clewis7 and kushalkolar as code owners March 29, 2024 14:24
@apasarkar
apasarkar marked this pull request as draft March 29, 2024 14:25
apasarkar added 3 commits March 29, 2024 16:10
@apasarkar
Fixes bug in kwarg initialization of frames_apply in imagewidget wher…
a77a41f
…e the function was only being applied to the first array
@apasarkar
In the _process_indices function the search for the array index in th…
18b423d
…e graphic now occurs outside of the for loop for running the window functions which is more efficient
@apasarkar
Enforces condition that we can only apply window function along T and…
d1f35fe
… Z dimensions, also removes logic saying that window_funcs can be ints, they now have to be dictionaries or None
kushalkolar
kushalkolar requested changes Mar 30, 2024
View reviewed changes
Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

apasarkar added 5 commits March 30, 2024 18:21
@apasarkar
Includes properly formatted updates from previous commit
3e539ca
@apasarkar
Updates some global variable names for clarity such as ALLOWED_DIMS a…
6bd6191
…nd also updates init docstring
@apasarkar
Updates doc strings and error messages for frame_apply and for error …
26d62c2
…handling in __init__
@apasarkar
Adds check that window function windowsize is an int and also simplif…
c9d1ade
…ies some of the slider dim logic to ensure all sliders come from t and z dims
@apasarkar
Removes some unreachable error statements, renames internal variables…
060161a
… to be clearer in the window funcs code, and updates some comments
kushalkolar
kushalkolar requested changes Apr 1, 2024
View reviewed changes
Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar

kushalkolar commented Apr 1, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I think we can use the a[np.newaxis] method as you mentioned, it returns views of the array which makes sense: a[np.newaxis, np.newaxis].ctypes.data == a.ctypes.data

Currently ImageWidget uses the lowest "t" and/or "z" dimension sizes to set the max bounds "_dims_max_bounds" for the ImageWidget and the sliders. If we instead set _dims_max_bounds based on the largest sizes in each dimension by modifying this bit:

fastplotlib/fastplotlib/widgets/image.py

Lines 540 to 546 in 5ec3fb2

# get max bound for all data arrays for all dimensions
self._dims_max_bounds: Dict[str, int] = {k: np.inf for k in self.slider_dims}
for _dim in list(self._dims_max_bounds.keys()):
for array, order in zip(self.data, self.dims_order):
self._dims_max_bounds[_dim] = min(
self._dims_max_bounds[_dim], array.shape[order.index(_dim)]
)

and then in @current_index.setter

fastplotlib/fastplotlib/widgets/image.py

Lines 218 to 221 in 5ec3fb2

for i, (ig, data) in enumerate(zip(self.managed_graphics, self.data)):
frame = self._process_indices(data, self._current_index)
frame = self._process_frame_apply(frame, i)
ig.data = frame

when iterating through each individual data array, we do something like this to prevent out of bounds indices for the data arrays which have fewer "t" or "z" items (ex. corr images) than the other data arrays (ex. movies).

current_index = dict()
for k in self._current_index:
  dim_ix = dict(reversed(ALLOWED_SLIDER_DIMS))[k]
  current_index[k] = min(self._current_index[k], data.shape[dim_ix])

It'll be generally useful anytime the shapes are very different. Might want to raise a warning in the constructor and set_data() to just let the user know that the array shapes are different and to expect this type of behavior (ex. sliding through data using the "t" slider will only change the graphic data if data exists at that position along that dimension, else it is ignored and the other data arrays are updated).

apasarkar reacted with eyes emoji
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

apasarkar added 3 commits April 2, 2024 11:28
@apasarkar
Includes support for mixed data types and RGB for imagewidget
ba6f5a4
@apasarkar
Simplifies the code by removing dims_partition and replacing it with …
0d217b0
…num_scrollable_dims
@apasarkar
Simplifies the imwidget api by replacing color_schemes with rgb_disp,…
1bc7190
… a list of indices which are to be displayed as rgba
@kushalkolar kushalkolar mentioned this pull request Apr 7, 2024
Gridplot -> Figure #479
Merged
7 tasks done
kushalkolar
kushalkolar requested changes Apr 9, 2024
View reviewed changes
Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

kushalkolar
kushalkolar requested changes Apr 9, 2024
View reviewed changes
Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

apasarkar added 7 commits April 9, 2024 16:03
@apasarkar
Removes old global variables and color scheme function
5b82074
@apasarkar
Renames global variables for dimension counts to be clearer, changes …
1002857
…num to n for readability, makes rgb init parameter a list of boolean values
@apasarkar
Adds extra check to make sure rgb input list for imagewidget is same …
3c3f0dc
…length as number of arrays being displayed
@apasarkar
Updates get_n_scrollable_dims function names and docstrings
54d8552
@apasarkar
Updates the function to get number of scrollable dimensions for each …
cfe4bd6
…array so it can be applied in a modular fashion to each individual array
@apasarkar
Updates the set_data code, removing hard to maintain constants, updat…
5ff549e
…ing the max length logic to reflect the mixed types and also adding explicit property for histogram_widget
@apasarkar
Fixes bug that did not update slider dimensions properly and also inc…
1d75b9b
…ludes linting
kushalkolar
kushalkolar requested changes Apr 10, 2024
View reviewed changes
Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

apasarkar and others added 7 commits April 9, 2024 21:54
@apasarkar
Updates documentation, removes unnecessary public variables
cbd47c2
@apasarkar
Merge branch 'main' into refactor_imwidget
2360142
@kushalkolar
fix indexing
5ce336f
@kushalkolar
fix test
8679910
@kushalkolar
Update image_widget_test.ipynb
db5f5b4
@kushalkolar
black
491aa1f
@apasarkar
Includes new testing for imagewidget rgb and mixed types
29ed277
@apasarkar

apasarkar commented Apr 11, 2024

Copy link
Copy Markdown
Collaborator Author

@kushalkolar It seems the tests for pre-existing cells in image_widget_test.ipynb are failing. The rgba_to_rgb function seems to complain that the input has 2 indices but 3 indices are indexed etc. Let me know what you think, maybe we need to update the rgba_to_rgb function?

Once we have that fixed, I can re-run to generate + update new screenshots.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar
kushalkolar marked this pull request as ready for review April 11, 2024 04:29
@kushalkolar

kushalkolar commented Apr 11, 2024

Copy link
Copy Markdown
Member

I've made this a full PR from the draft so the CI will run on GitHub actions, we can check the result tomorrow.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

apasarkar added 3 commits April 11, 2024 15:41
@apasarkar
Overrides synced controllers for gridplot imagewidget test
cf3cba1
@apasarkar
Includes black after updating the tests
eba97f7
@apasarkar
Includes histogram widget in the new tests for rgb and mixed types
8fa21de
kushalkolar
kushalkolar reviewed Apr 12, 2024
View reviewed changes
Comment thread fastplotlib/widgets/image.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@kushalkolar
Apply suggestions from code review
aea6d56
@kushalkolar
kushalkolar changed the base branch from main to iw_refactor April 12, 2024 04:28
@kushalkolar
kushalkolar merged commit 55e8b4e into fastplotlib:iw_refactor Apr 12, 2024
@kushalkolar kushalkolar mentioned this pull request Apr 12, 2024
Iw refactor #482
Merged
kushalkolar added a commit that referenced this pull request Apr 12, 2024
@kushalkolar @apasarkar
Iw refactor (#482)
e7d940d
* fefactor imwidget, restrict to tzxy, txy, xy, added RGB(A) support (#459)

* add new iw test screenshots

* update api docs

---------

Co-authored-by: Amol Pasarkar <[email protected]>
kushalkolar added a commit that referenced this pull request Apr 12, 2024
@kushalkolar @clewis7 @apasarkar
Gridplot -> Figure (#479)
25aa9e3
* figure refactor seems to work

* update nbs w.r.t. new figure

* update test util

* black

* remove lingering uses of gridplot

* update API doc generation

* update contributing

* typo

Co-authored-by: Caitlin Lewis <[email protected]>

* Iw refactor (#482)

* fefactor imwidget, restrict to tzxy, txy, xy, added RGB(A) support (#459)

* add new iw test screenshots

* update api docs

---------

Co-authored-by: Amol Pasarkar <[email protected]>

* figure refactor seems to work

* update nbs w.r.t. new figure

* update test util

* black

* remove lingering uses of gridplot

* update API doc generation

* update contributing

* typos

* update iw with FIgure

* update api docs

* black

* update quickstart

* update iw examples

* update iw type annotations

* default cmap iw

* remove gp docs

* update readme

* forgot to delete more api docs

* I need to run nbs before committing them

* more docs

---------

Co-authored-by: Caitlin Lewis <[email protected]>
Co-authored-by: Amol Pasarkar <[email protected]>
@apasarkar
apasarkar deleted the refactor_imwidget branch July 15, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kushalkolar kushalkolar kushalkolar requested changes
@clewis7 clewis7 Awaiting requested review from clewis7

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor ImageWidget

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@apasarkar @kushalkolar
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.