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

migrate from Appveyor to GitHub Actions #338

Merged
merged 89 commits into from
Mar 6, 2021
Merged

Conversation

StrikerRUS
Copy link
Member

Fixed #122.
Appveyor suggests only 1 parallel job at free tier, GitHub Actions - 20.

Should be considered as a continuation of #328. Same changes as for *nix OSes: latest R version; stop producing 32bit artifacts.

@StrikerRUS
Copy link
Member Author

@fukatani Could you please give a second review for this PR?

Copy link
Member

@fukatani fukatani left a comment

Choose a reason for hiding this comment

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

LGTM

@StrikerRUS StrikerRUS merged commit ab328a0 into master Mar 6, 2021
@StrikerRUS StrikerRUS deleted the github_actions_windows branch March 6, 2021 12:13
@StrikerRUS
Copy link
Member Author

StrikerRUS commented Mar 6, 2021

Thank you guys for reviews!

@fukatani
Could you please help with new release on PyPI?

@mlampros
Could you please help with new release on CRAN?

@mlampros
Copy link
Collaborator

mlampros commented Mar 6, 2021

Yes. Can you please point me to what was changed in the code base so that I can adjust the R code? Thanks for the notification.

@StrikerRUS
Copy link
Member Author

I think the only change in R-package was in Python version requirements:
9e558f9#diff-99c1b03acafa469ec3315aee841d2b64f122907ff1ebef7e7be5c793faf5bf0d
cd99ce8#diff-99c1b03acafa469ec3315aee841d2b64f122907ff1ebef7e7be5c793faf5bf0d

I don't think any code adjustments are needed because all R tests are passed in the latest master branch.

If you think that requirements in Python version is too minor to have new release on CRAN, feel free to skip it.

@mlampros
Copy link
Collaborator

mlampros commented Mar 7, 2021

I think the drop of support for Python 2.7 (version) has to be added in the DESCRIPTION file of the CRAN package. I'll submit the updated version tomorrow to CRAN and I'll notify you once I received the approval.

@StrikerRUS
Copy link
Member Author

Hey @mlampros !
Are there any problems with CRAN acceptance? Now I'm familiar a little bit with CRAN and can help you in case of any additional requests from CRAN administration.

@mlampros
Copy link
Collaborator

mlampros commented Mar 14, 2021

Hi, @StrikerRUS thanks for asking.

I had indeed a bunch of problems when submitting the package:

The problems were related with a namespace error as is also discussed in this issue and appeared mainly in the Windows version (I received 2 NOTES compared to the last submission of the package). It took me a while (I think 5 or 6 submissions) to figure out that it was related with the package.R file only and not with the tests, however it seems that it might be a good practice to not run the tests on CRAN . Finally, I ended up removing the initialization of Python from the .onload() function.

Anyway, now it works for all operating systems, however CRAN included lately tests also for M1mac (Checks on a M1 (arm64) Mac). I think I fixed the error by adjusting a lot of code in the tests and examples section but currently it's not a requirement for a successful submission to CRAN (probably it's in a testing phase).

I had to reply that I fixed the errors and now I'm awaiting for a response from the CRAN personnel. Once I receive the confirmation I'll make a pull request by uploading the new version.

@StrikerRUS
Copy link
Member Author

@mlampros Thank you very much for the detailed report!

Anyway, now it works for all operating systems

Great! Thank you!

however CRAN included lately tests also for M1mac (Checks on a M1 (arm64) Mac). I think I fixed the error by adjusting a lot of code in the tests and examples section but currently it's not a requirement for a successful submission to CRAN (probably it's in a testing phase).

Yeah, I think this is beta test when CRAN picks up projects randomly and runs tests for them. For example, there is no M1mac point in the results for LightGBM: https://cran.r-project.org/web/checks/check_results_lightgbm.html.
I see the error is in scipy.sparse-related tests

  ══ Failed tests ════════════════════════════════════════════════════════════════
  ── Error (test-RGF_package.R:265:3): the 'mat_2scipy_sparse' returns a scipy sparse matrix ──
  Error: attempt to apply non-function
  Backtrace:
      █
   1. └─RGF::mat_2scipy_sparse(x_rgf, format = "sparse_row_matrix") test-RGF_package.R:265:2

I remember you told that there are some problems with reticulate integration with scipy.sparse on macOS. Maybe it is enough just to adjust the following condition?

if (Sys.info()["sysname"] != 'Darwin') {
# conversion of an R 'dgCMatrix' and 'dgRMatrix' to a scipy sparse matrices
#--------------------------------------------------------------------------
testthat::test_that("the 'TO_scipy_sparse' returns an error in case that the input object is not of type 'dgCMatrix' or 'dgRMatrix'", {

@StrikerRUS
Copy link
Member Author

Kindly ping @fukatani for a new PyPI release.

@StrikerRUS
Copy link
Member Author

@fukatani CRAN release has been published recently: #340 (comment). Please help with PyPI one to sync packages (Python 3.9 support, removed Python 2.7 support, compatibility with new scikit-learn, etc.).

@StrikerRUS
Copy link
Member Author

@fukatani If you'd like, I can take the responsibility to make releases on PyPI. My nickname there is the same as on GitHub.

@fukatani
Copy link
Member

@StrikerRUS
Sorry for late response.
Unfortunately, my OSS contribution is inactive in current.
I am grateful for the contributions of you and mlampros.

I'm investigating how to transfer PyPI authority.

@StrikerRUS
Copy link
Member Author

StrikerRUS commented Apr 20, 2021

@fukatani

Unfortunately, my OSS contribution is inactive in current.

Totally undestand!

I'm investigating how to transfer PyPI authority.

No need to transfer, you can simply invite me to collaborate and I'll be able to upload new releases.

Go to "Your projects" at PyPI site: https://pypi.org/manage/projects/.

Click "Manage" button for the rgf-python project.

Select "Collaborators" tab at the left side of the page.

Scroll down to the "Invite collaborator" section.

My "Username" there is the same as at GitHub.

image

@fukatani
Copy link
Member

@StrikerRUS
Thx!
I invited you as maintainer.
Could you confirm it?

@StrikerRUS
Copy link
Member Author

@fukatani Thank you so so much! Just accented your invite. Will try to upload new release in a few days.

@StrikerRUS
Copy link
Member Author

New release is out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Reduce test job time
3 participants