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

Patch utils: tentative v0.3. #11

Merged
merged 15 commits into from
Feb 27, 2023
Merged

Patch utils: tentative v0.3. #11

merged 15 commits into from
Feb 27, 2023

Conversation

cako
Copy link
Collaborator

@cako cako commented Jan 22, 2023

Description

This PR adds several features and minor improvements. Three new modules are created: plot, utils, typing.

  • plot: Contains functions for plotting curvelet coefficients in different forms, including curveshow which shows all curvelet coefficients in space of frequency domain, and overlay_disks, which visualizes the energy of curvelet coefficients in different pars of the image in a similar way to the Kymatio scattering disk example. These plotting functions require the creation of specializes axes and grids, so convenience functions have been added, see new functions create_colorbar, create_axes_grid and create_inset_axes_grid, for example. Finally, another useful function to overlay a vector field in a similar manner to overlay_disks is included: overlay_arrows. All of the plotting functions are thoroughly documented with self-contained examples.
  • utils: Contains functions which are helpful when dealing with curvelet structures. apply_along_wedges, for example, maps a lambda function to each curvelet coefficient organized in a curvelet structure. energy is an utility function for computing the energy of a single wedge. Putting these two together can create a structure of curvelet energies. Another useful utility function array_split_nd, splits an N-dimensional array in chunks for each dimension. For example, if you want to split a 2D array into 4 quadrants, you can do array_split_nd(two_d_array, 2, 2). This function also handles uneven splits. It is essentially the N-dimensional version of NumPy's array_split. A similar function that only supports even splits is also included, split_nd.
  • typing: Contains some useful classes for type annotation: FDCTStructLike, a list of list of nd-arrays and RecursiveListNDArray, a recursive list of nd-arrays.

Suggestion for review

Clone, pip install -e ., run pytest and make doc; make servedoc. From the docs, run the examples in the documentation in an IPython/Jupyter session. Ignore the changes to the cpp files: it is just a reformatting. Run flake8 curvelops/ and mypy curvelops/.

@cako cako added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 22, 2023
@cako cako requested a review from mrava87 January 22, 2023 21:50
@mrava87
Copy link
Contributor

mrava87 commented Feb 5, 2023

This looks very good. A few comments based on my process going through what you suggested:

  • There doesn't seem to exist any requirement/environment for developers, so whilst I can easily install myself the required libraries for tests and doc, it would be good to have an easy to install env like in pylops.
  • The documentation doesn't seem to have working examples right? Just code snippets. For now I think it's ok but it is a bit tedious to copy and paste examples and run them (unless I am missing something). I did it for some of them and didn't find any error. Moving to sphinx-gallery is probably a good idea.
  • Not sure what you plan to do with the doc (host it on Github pages and build it locally? I do the same in PyMarchenko due to heavy compute on some examples...) or let the user just build it for themselves?

Apart from these comments, feel free to go ahead and merge at any time :)

@cako
Copy link
Collaborator Author

cako commented Feb 27, 2023

Thanks @mrava87, I added the requirements and improved the Makefile to go along with it. Regarding your other comments, I think it would be a good idea to host it in github pages, what do you think? And I agree with the sphinx-gallery, as right now the examples are just a collection of notebooks. I will try to figure out the sphinx-gallery in the coming days (new issue #12). Could you investigate how the github pages would work within the PyLops namespace and add any recommentations here: #13?

Feel free to merge this if you are happy with it.

@mrava87
Copy link
Contributor

mrava87 commented Feb 27, 2023

Perfect, let me reply here then I merge:

For GitHub pages this is how I set it up for PyMarchneko: https://github.com/DIG-Kaust/pymarchenko/blob/main/Makefile (see the last two commands and this https://github.com/DIG-Kaust/pymarchenko/blob/main/docssrc/Makefile

The assumption here is that you cannot build the doc anywhere but locally (so not even with a GitHub action, which I think is the case for curvelops since we can’t really download the Curvelets code in CI). Also this isn’t really my own solution, I found it somewhere online but it has worked well for me :)

The other step is to go to Settings -> Pages of this repo and select Deploy from a branch and choose the gh-pages

@mrava87
Copy link
Contributor

mrava87 commented Feb 27, 2023

Of course I am happy to help setting it up :)

@mrava87 mrava87 merged commit 3e27fe9 into main Feb 27, 2023
@cako cako deleted the patch-utils branch February 28, 2023 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants