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

v1.0 release candidate #42

Closed
wants to merge 53 commits into from
Closed

v1.0 release candidate #42

wants to merge 53 commits into from

Conversation

ablack3
Copy link
Collaborator

@ablack3 ablack3 commented Nov 21, 2022

This release candidate switches the Andromeda backend from SQLite to Apache Arrow. It is not technically a breaking change but due to slight differences in how SQLite tables and arrow tables are handled it can't be released until it is tested with all reverse dependencies.

Addresses

Adam Black and others added 30 commits January 22, 2021 10:35
Migrate from travis ci to github actions
Explicitly import external class definitions and add code coverage to github actions
Change package maintainer to Adam Black and update github actions yaml, compare_versions, and deploy.sh
* Add native date and datetime support.

* Add hms to Imports. Needed for dates in RSQLite.

Co-authored-by: Adam Black <[email protected]>
Change broken link to pdf vignette to html vignette.
…iffer only by case. Prior implementation changed table order.
@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Patch coverage: 78.23% and project coverage change: -6.78% ⚠️

Comparison is base (6c679dc) 89.21% compared to head (db6c2db) 82.44%.
Report is 4 commits behind head on main.

❗ Current head db6c2db differs from pull request most recent head 279c07d. Consider uploading reports for the commit 279c07d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
- Coverage   89.21%   82.44%   -6.78%     
==========================================
  Files           4        3       -1     
  Lines         371      262     -109     
==========================================
- Hits          331      216     -115     
- Misses         40       46       +6     
Files Changed Coverage Δ
R/Object.R 71.92% <69.14%> (-14.28%) ⬇️
R/Operations.R 88.88% <80.00%> (-0.74%) ⬇️
R/LoadingSaving.R 92.53% <91.52%> (+7.14%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…eda tables. try to delete locked files later.
@ablack3
Copy link
Collaborator Author

ablack3 commented Mar 16, 2023

@schuemie,

I've made the changes we discussed in the call today.
nrow, isAndromedaTable, and colnames should now work with both old and new Andromeda tables. Tests have been added.

I also added some code to try and handle the windows file lock issue. Not sure how to test it because it is kind of a random thing and I have no way to know when the lock is released if ever. But maybe it will help. Each time close is called on any Andromeda I try to remove not only the one Andromeda that it is being called on but also any others that are lingering around.

I don't know what to do about the upcoming changes to pull except maybe to complain to the Arrow folks. Otherwise we can just ask users to set an option. We could check the option on startup and print a message.

One thing I wanted to ask about since this is a big change. I'd like to consider moving dplyr from Depends to Imports. The reason is that I'm exporting a lot (all of dplyr), some of which has nothing to do with Andromeda or doesn't work on Andromeda. What if we asked users to load dplyr if they want it? Or possibly just re-export a smaller subset of dplyr functions?

@schuemie
Copy link
Member

Would it be possible to do a small release first that introduces the nrow() and isAndromedaTable() first, but doesn't change the backend to arrow? That way, we can have all HADES packages require that new version and use those functions. That should make it a lot easier to make the HADES packages work with both backends.

As for dplyr, I don't feel very strongly about it. I tend to rather aggressively import all of dplyr in my packages anyway, like here, so I would not be affected by Andromeda moving it to Imports. I guess to me, dplyr has become part of the R base, I just always have it loaded.

@ablack3
Copy link
Collaborator Author

ablack3 commented Mar 20, 2023

Would it be possible to do a small release first that introduces the nrow() and isAndromedaTable() first, but doesn't change the backend to arrow? That way, we can have all HADES packages require that new version and use those functions. That should make it a lot easier to make the HADES packages work with both backends.

Yes I did create a release with isAndromedaTable. I didn't add nrow yet but I can to that this week in another mini release.

As for dplyr, I don't feel very strongly about it. I tend to rather aggressively import all of dplyr in my packages anyway, like here, so I would not be affected by Andromeda moving it to Imports. I guess to me, dplyr has become part of the R base, I just always have it loaded.

Ok. I think I might try moving it to Imports and only export the dplyr functions supported by Andromeda (select, mutate, etc).

egillax and others added 2 commits May 15, 2023 09:43
* Using ParallelLogger's JSON functions to store attributes. Fixes #54

* Dropping jsonlite from DESCRIPTION

---------

Co-authored-by: Admin_mschuemi <[email protected]>
@ablack3 ablack3 closed this Nov 24, 2023
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.

5 participants