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 1013 #1018

Merged
merged 17 commits into from
Jan 20, 2021
Merged

Fix 1013 #1018

merged 17 commits into from
Jan 20, 2021

Conversation

LeilyR
Copy link
Contributor

@LeilyR LeilyR commented Nov 16, 2020

inspired by #1013

@LeilyR
Copy link
Contributor Author

LeilyR commented Dec 1, 2020

Hi @dpryan79, sorry to bother you. Could you please have a look at this. I think it needs to be fixed under account. Let me know if there anything I can do for it. Thanks a lot!

@dpryan79
Copy link
Collaborator

dpryan79 commented Dec 3, 2020

@LeilyR I'll see what I can do

@LeilyR
Copy link
Contributor Author

LeilyR commented Dec 3, 2020

@dpryan79 Thanks a lot

@LeilyR
Copy link
Contributor Author

LeilyR commented Dec 3, 2020

Just to let you know, this pc2 value changes frequently between those two numbers, sometime is 1.2325951644078315e-32 some other time is 2.49319462166397e-32. I have seen both of them.

@dpryan79
Copy link
Collaborator

dpryan79 commented Dec 3, 2020

Yeah, I think it depends on the CPU that gets used.

@dpryan79
Copy link
Collaborator

dpryan79 commented Dec 3, 2020

@LeilyR can you check computeMatrixOperations locally? It's running rbind and I have no clue why it's immediately throwing an error.

@LeilyR
Copy link
Contributor Author

LeilyR commented Dec 3, 2020

@dpryan79 I have already tried it locally and it fails there too. I was also really confused, since it says it cannot find the info command. with the updated galaxy version also master branch fails for this test, Though I could get to run the 3.4.3 version locally with no issue. 3.4.3 is the latest version before introducing dataRange. What I do not understand is that if that is the problem why rbind test passes. Also info used to pass before we change the galaxy version. I will have a look at it again, but if you also have any opinion I would be happy to hear. Again thank a million for looking into it.

@dpryan79
Copy link
Collaborator

dpryan79 commented Dec 3, 2020

I mean on the command line, does it run properly there with computeMatrixOperations rbind ...? The data is under galaxy/wrapper/test-data.

@LeilyR
Copy link
Contributor Author

LeilyR commented Dec 3, 2020

I see ok thanks , i will check

@LeilyR
Copy link
Contributor Author

LeilyR commented Jan 19, 2021

Hi @dpryan79 , I got to fix the galaxy issues. However there is an issue with the tests running under your account. The error is:
Error: Theadd-pathcommand is disabled. Please upgrade to using Environment Files or opt into unsecure command execution by setting theACTIONS_ALLOW_UNSECURE_COMMANDSenvironment variable totrue. For more information see: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/
Could you please update this? Do you think I should merge my PR despite those failures?

@dpryan79
Copy link
Collaborator

The azure tests are all green so it's safe to merge. I'll see if I can get the environment variable set properly. Otherwise I'll just update the nodejs stuff, which should get migrated to the deeptools repo so others can update it more easily.

@dpryan79
Copy link
Collaborator

It only fails on the annoying plotPCA test. Long-term we might just remove that, since it results only in false positive errors.

@LeilyR
Copy link
Contributor Author

LeilyR commented Jan 20, 2021

thanks a lot @dpryan79 ! I will merge it then :)

@LeilyR LeilyR merged commit d560650 into develop Jan 20, 2021
@LeilyR LeilyR deleted the fix_1013 branch January 20, 2021 08:03
This was referenced Jan 20, 2021
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