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

Skip spatial LDA visualization tests due to Numpy error #984

Merged
merged 7 commits into from
May 3, 2023

Conversation

alex-l-kong
Copy link
Contributor

@alex-l-kong alex-l-kong commented May 2, 2023

What is the purpose of this PR?

The spatial LDA visualization tests currently imports spatial_lda.visualization and spatial_lda.processing. There's a bug at https://github.com/calico/spatial_lda/blob/primary/spatial_lda/online_lda.py#L31 where an incompatible np.float call is made. This is outdated with the most up-to-date version of numpy and throws an error on import.

How did you implement your changes

We'll have to check in with calico to get this resolved, but in the meantime, pin numpy back at 1.23, the last version that doesn't throw an error.

Remaining issues

Not sure how long calico will take to respond. They don't seem particularly invested. Who was Brian's point of contact on this?

@alex-l-kong alex-l-kong changed the title Skip analysis.test_visualize_fov_graphs test due to Numpy error Skip spatial LDA visualization tests due to Numpy error May 2, 2023
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

They'll never respond, it's fine to just pin for now. Once this version gets deprecated, we'll know if we're still using spatialLDA or not.

@ngreenwald ngreenwald merged commit 061a128 into main May 3, 2023
@ngreenwald ngreenwald deleted the fix_np_float branch May 3, 2023 23:19
@srivarra srivarra added the dependencies Pull requests that update a dependency file label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants