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

Fix issues with docstring of gtda.mapper.method_to_transform #396

Merged
merged 1 commit into from
Apr 14, 2020

Conversation

ulupo
Copy link
Contributor

@ulupo ulupo commented Apr 13, 2020

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description
The current docstring example for the method_to_transform decorator in gtda.mapper became incorrect following changes that reshape the output of the new transform method to ensure it is at least 2D. This PR documents this reshaping in the docstring and fixes the example.

Checklist

  • I have read the guidelines for contributing.
  • My code follows the code style of this project. I used flake8 to check my Python changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed. I used pytest to check this on Python tests.

@ulupo ulupo requested a review from wreise April 14, 2020 07:21
Copy link
Collaborator

@wreise wreise left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@ulupo ulupo merged commit de22918 into giotto-ai:master Apr 14, 2020
@ulupo ulupo deleted the fix_method_to_transform branch April 25, 2020 15:29
ulupo added a commit that referenced this pull request Jun 2, 2020
* Fix issue with docstring example and document reshaping of 1D outputs (#396)

* Toc (#394)

* Update P landscapes

Signed-off-by: ammedmar <anibal@medina-mardones.com>

* Add distances, inner products and kernels glossary entry

Signed-off-by: ammedmar <anibal@medina-mardones.com>

* Remake vectorization changes

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Change [] for \lbrack \rbrack

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after W's comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after W's comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update afte W's second comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after Umbe's comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after Umbe's second comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after Umbe's third comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after Umbe's 4th comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Remove concept k-skeleton

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Add table of content

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Add Lp & lp. Update landscape

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update TOC indentation

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Add heat vectorizations entry

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update indentation of TOC

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Fix extra spacing in bibliography

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update bibliography hack for caps

Signed-off-by: ammedmar <ammedmar@gmail.com>

* After Umbe's Comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after issue #398

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after Umbe's comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

* Update after Umbe's 2nd comments

Signed-off-by: ammedmar <ammedmar@gmail.com>

Co-authored-by: ammedmar <anibal@medina-mardones.com>

* Speedup windows pipeline (#402)

* Improve boost location for azure pipeline on windows

The boost version installed in the pipeline is now used

Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
Co-authored-by: Umberto Lupo <46537483+ulupo@users.noreply.github.com>

* Make bindings public (#395)

* Make bindings public

Signed-off-by: Guillaume Tauzin <guillaumetauzin.ut@gmail.com>

* Fix pipeline on Mac (#407)

* Refresh ccache

* Enforce CXX standard to 14 on each module

* Change variable name to comply with E741

Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
Co-authored-by: Umberto Lupo <46537483+ulupo@users.noreply.github.com>

* Mapper visualisation refactor: fix bugs, add summary statistics in hovertext, improve opacity, remove matplotlib dependency, add node_scale kwarg, add clone_pipeline kwarg to interactive plots,restructure/rename plotly_kwargs, improve code (#406)

* Refactor of `mapper/visualisation.py` and `mapper/utils/visualisation.py`
- Removal of color scaling
- More modularity
- Variable and function name changes
- Remove matplotlib functions used for hoverlabel background color
- Remove cmin and cmax
- Change return signature of some functions

* Add test-output.xml to .gitignore

* Replace "text" key in plot_options["node_trace"] with "hovertext"

* Improve width and opacity of nodes and edges

* Display summary statistics in hovertext with significant figure rounding via new n_sig_figs kwarg

* Rename plotly_kwargs to plotly_params

* Remove matplotlib installation requirement

* Fix mapper notebook

* Add small comment on use of matplotlib in voids_on_the_plane

* Place matplotlib in examples requirement in setup.py

* Improve docstring of make_mapper_pipeline

* Improve examples for make_mapper_pipeline and create one for plot_static_mapper_graph

* Hide set_node_sizeref, add node_scale kwarg, add clone_pipeline kwarg to plot_interactive_mapper_graph

- Update docstrings
- Update quickstart notebook

* Add helper function for colorscale-based interpolations, improve use of plotly_params

Static and interactive plots display the expect hoverlabel colors in 3D, or white if things go wrong. Thanks to @lewtun for pointing out that this was needed.

* Improve docstrings

* Hide visualization module in mapper/utils

* Add pip install matplotlib to notebook tests in manylinux job (#410)

* Fix mapper docstring issues following #406 (#411)

* Fix issues with mapper docs following #406

* Create giotto-tda version 0.2.2 (#413)

* Turn CODE_OWNERS and CODE_AUTHORS into an rst file

* Bump version number to 0.2.2

* Add release notes for v0.2.2

Co-authored-by: Anibal M. Medina-Mardones <ammedmar@gmail.com>
Co-authored-by: ammedmar <anibal@medina-mardones.com>
Co-authored-by: REDS institute <reds-heig@users.noreply.github.com>
Co-authored-by: Guillaume Tauzin <guillaumetauzin.ut@gmail.com>
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.

2 participants