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 misc.py import for confusion matrix plot #53

Merged
merged 7 commits into from
Nov 15, 2023
Merged

Conversation

bruce-edelman
Copy link
Contributor

This fixes #52 by changing the import statement for misc.py to plot confusion matrix from the main diploSHIC script

@andrewkern
Copy link
Member

LGTM, but we are getting some error with the actions. any clue what's up with that?

@bruce-edelman
Copy link
Contributor Author

LGTM, but we are getting some error with the actions. any clue what's up with that?

Weird -- it is failing to build numpy when doing a pip install to build the wheel. It shows that the version of gcc on the machine it is running on to be 8.3.1 but requires >= 8.4 to build numpy. I think updating the docker image that the wheels are built in will fix this. I will try that out and add that to this PR

@bruce-edelman
Copy link
Contributor Author

okay this should now be good to go @andrewkern

Yesterday was weird as I updated the manylinux docker image to one with newer gcc versions and the build passed once. Afterwards it failed many times over from a weird error downloading the remote docker image.

Today after just rerunning the CI it seems to have worked again so there must have been server issues yesterday that are now fixed. The reason the pull_request version of the PyPI upload failed was because a wheel for version 1.0.5 already exists from the "push" version of the PyPI upload job that succeeded.

Not that it is too important but I think in my PR with domain adaptation I had tried to split up the github action jobs so that the PyPI uploading only happens once and thus doesn't cause subsequent jobs with same version numbers to fail.

@andrewkern
Copy link
Member

okay sounds good @bruce-edelman -- I'll merge this. what do you reckon we should do with the domain adaptation PR? Should we split out the action edits to another PR?

@bruce-edelman
Copy link
Contributor Author

Sure that sounds good to me @andrewkern -- I do that and create another PR with the changes to the actions

@andrewkern andrewkern merged commit a606f98 into master Nov 15, 2023
3 of 4 checks passed
@andrewkern
Copy link
Member

arg... looks like the upload to pypi failed, presumably because the version number needs to be bumped?

@bruce-edelman
Copy link
Contributor Author

arg... looks like the upload to pypi failed, presumably because the version number needs to be bumped?

I actually think it worked. Its just currently configured to run twice. Once on the push and once for the pull request -- so once completed and then the second failed because there was already a v1.0.5 published to PyPI -- diploshics PyPI page showed 1.0.5 recently uploaded

@andrewkern
Copy link
Member

okay awesome!

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.

Error printing out confusion matrix
2 participants