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

Updating the examples files #63

Merged
merged 1 commit into from
Jul 8, 2022
Merged

Updating the examples files #63

merged 1 commit into from
Jul 8, 2022

Conversation

alexdepremia
Copy link
Collaborator

Proposed changes

Some of the example files are outdated. Some of them are not working anymore.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • [ x] Bugfix (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 not work as expected)

Checklist

Put an x in the boxes that apply.

  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works

@alexdepremia alexdepremia requested a review from AldoGl June 30, 2022 12:44
@alexdepremia
Copy link
Collaborator Author

When executing the examples notebooks, there are some problems. I solved the ones in the intrinsic dimension and the clustering notebooks. The information imbalance code gives me an error when dealing with periodic boundary conditions, but I think it is related to the cKDTree function in Scipy.
Error message:
File ckdtree.pyx:550, in scipy.spatial.ckdtree.cKDTree.init()
IndexError: invalid index to scalar variable.

Can someone check if you have the same error? My Scipy version is 1.7.1

@imacocco
Copy link
Collaborator

I just run the whole inf-imb notebook with both scipy 1.7.3 and scipy 1.8.1 and everything went smooth

@AldoGl
Copy link
Collaborator

AldoGl commented Jul 5, 2022

They work for me too, I have scipy 1.6.1 and numpy 1.22.3

@AldoGl
Copy link
Collaborator

AldoGl commented Jul 7, 2022

@alexdepremia if you agree I would merge all your changes excluding the changes to the intrinsic dimension notebook (because these conflict with some major improvements that @diegodoimo recently implemented). Let me know

@alexdepremia
Copy link
Collaborator Author

@AldoGl , yes sure. Go ahead!

@AldoGl
Copy link
Collaborator

AldoGl commented Jul 8, 2022

@AldoGl , yes sure. Go ahead!

Done it, merging after the tests pass

@AldoGl AldoGl merged commit f6a68a3 into main Jul 8, 2022
@AldoGl AldoGl deleted the test_examples branch July 8, 2022 13:20
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

3 participants