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

Rectified Issue #11 #13

Merged
merged 7 commits into from
Jan 20, 2021
Merged

Rectified Issue #11 #13

merged 7 commits into from
Jan 20, 2021

Conversation

Aggarwal-Ayush
Copy link
Contributor

Now aliases are mapped correctly whenever chromosome is specified even when the gene is correct on another chromosome.

@lwaldron
Copy link
Member

Thanks @Aggarwal-Ayush! Could you take a look at the cause of the failed checks? And, would you add a unit test that confirms your fix is working, e.g. it could use code I posted on the issue #11

@Aggarwal-Ayush
Copy link
Contributor Author

Yes, I'm looking into it. I'll add the unit tests after I determine the cause of the failed checks.

@Aggarwal-Ayush
Copy link
Contributor Author

Hi @lwaldron! Somehow the hgnc.table in the package does not contain the column for chromosome now and I think that is what is causing the checks to fail. Could you please take a look at it?

@lwaldron
Copy link
Member

yes, will do.

@lwaldron
Copy link
Member

@Aggarwal-Ayush check your HGNChelper version - 0.8.1 in CRAN does not have the chromosome column, but the current version on GitHub (0.8.7) does (you can check yourself in the data/ folder). But it means that isn't the reason for your commit failing, since GHA installs and checks the current version from GitHub.

@Aggarwal-Ayush
Copy link
Contributor Author

@lwaldron Yes, my mistake. I updated to the latest version from GitHub and then ran the test_checkGeneSymbols.R manually in RStudio. It is passing all the test except for the mouse test 2. In that, the correct symbol specified for "Cpamd8" is "Cpamd8" whereas for "cpamd8" it is "Mug2" and for "mug2" it is "Cpamd8 /// Mug2" while for "Mug2" it is "Mug2". Is this the expected behaviour?
Also, I've added the unit test for my current fix under "human test 3 - chromosome".

@lwaldron
Copy link
Member

@shbrief can you say what is the correct behavior for this mouse test? While you're checking - can you check whether there is a chromosome column available with the mouse map download?

@shbrief
Copy link
Contributor

shbrief commented Jan 20, 2021

@Aggarwal-Ayush I can't reproduce the error with the mouse test 2 - all tests were passed from my end. Is it possible that your mouse data file is corrupted? Are you using GitHub LFS?

@lwaldron From the latest download, I didn't see a chromosome column in the mouse map.

@lwaldron lwaldron merged commit 2d42d2b into waldronlab:master Jan 20, 2021
@lwaldron
Copy link
Member

Alright, I merged the PR, which should make it a little easier to make sure everyone is testing the same version. I'll also bump the version number to 0.8.8 in a moment.

@lwaldron
Copy link
Member

Note, I also don't see any failing mouse unit test in GHA: https://github.com/waldronlab/HGNChelper/runs/1733750433?check_suite_focus=true

@Aggarwal-Ayush
Copy link
Contributor Author

Yes, that was an error on my part only. I fixed it in the last commit. This error is "undefined columns selected" which may be referring to one of my code line although I'm not sure because my latest commit was passing all the tests in Rstudio(latest verion).

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.

3 participants