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.

Sign up for GitHub

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

Jump to bottom

Fix pipeline on Mac #407

Merged
merged 3 commits into from
May 12, 2020
Merged

Fix pipeline on Mac #407

merged 3 commits into from
May 12, 2020

Conversation

reds-heig
Copy link
Collaborator

Reference issues/PRs

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

This PR will fix current issue encountered due to pipeline update the XCode compiler (actions/virtual-environments#777)

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.

Refresh ccache's in the pipeline
bd43d88 
Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
@CLAassistant
Copy link

CLAassistant commented May 12, 2020 •
edited
Loading

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ulupo
❌ MonkeyBreaker
You have signed the CLA already but the status is still pending? Let us recheck it.

Enforce CXX standard to 14 on each module
7222e79 
Signed-off-by: julian <julian.burellaperez@heig-vd.ch>
@MonkeyBreaker
Copy link
Collaborator

@ulupo can you please look why it fails on Mac now ?
Apparently the flake8 has an issue ? Not sure I understand why

@ulupo
Copy link
Contributor

ulupo commented May 12, 2020

@MonkeyBreaker this seems to be due to a new release of flake8 (3.8.1) made 4 hours ago! I have make a change to help with the situation.

@MonkeyBreaker
Copy link
Collaborator

@ulupo great !

I don't know why sometimes the pipeline doesn't show all the step ...
But, every step has run smoothly, I think we can merge this PR.

@MonkeyBreaker MonkeyBreaker changed the title WIP: Fix pipeline on Mac Fix pipeline on Mac May 12, 2020
@ulupo
Copy link
Contributor

ulupo commented May 12, 2020

@MonkeyBreaker thanks! Out of curiosity, is there a plan to fix the warnings which seem to have caused this issue, at some point?

@ulupo ulupo merged commit c5bae04 into giotto-ai:master May 12, 2020
@MonkeyBreaker
Copy link
Collaborator

@ulupo I'm not sure I understand about which warnings are you talking about

@ulupo
Copy link
Contributor

ulupo commented May 12, 2020

@MonkeyBreaker I'm referring to what seems to have caused problems before, namely https://dev.azure.com/maintainers/Giotto/_build/results?buildId=2082&view=logs&j=e1e30141-6252-56cf-eccb-600499366ae7&t=d9621201-b199-5a6f-a2d0-2aa31b7b1f05&l=663. It seems to me the fix is effectively to silence these warnings/errors?

@MonkeyBreaker
Copy link
Collaborator

Yes, the issue was that the updated compiler for Mac, requires to pass the standard version (In our case c++14). And because we didn't pass any, it raised warnings about using features for c++11 and of course created errors because the standard was not set to at least c++11.

@ulupo
Copy link
Contributor

ulupo commented May 12, 2020

the standard was not set to at least c++11

Do you mean "at least C++14"?

@MonkeyBreaker
Copy link
Collaborator

No, c++11, that's what the warnings has been printing. I aligned with pyflagser because pyflagser requires c++14.

ulupo added a commit that referenced this pull request Jun 2, 2020
@ulupo @ammedmar
@reds-heig @gtauzin
Create giotto-tda v0.2.2 (#414)
a0f709b 
* 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
Reviewers

@ulupo ulupo ulupo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@reds-heig @CLAassistant @MonkeyBreaker @ulupo