-
Notifications
You must be signed in to change notification settings - Fork 22
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
Support keyword arguments in CallJuliaFunctionWithCatch
, and a few other improvements
#1043
Conversation
(motivated mainly by gap-packages/PackageManager/pull/134 and the issues mentioned there) - support keyword arguments in `CallJuliaFunctionWithCatch` - do not use the deprecated and undocumented `Core._apply` - update the initializations in `init_packagemanager`: Replacing `PKGMAN_DownloadURL` is no longer needed, replacing `PKGMAN_RemoveDir` was in a wrong place. - start better documentation of Julia syntax features
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1043 +/- ##
==========================================
+ Coverage 74.39% 74.44% +0.05%
==========================================
Files 55 55
Lines 4527 4540 +13
==========================================
+ Hits 3368 3380 +12
- Misses 1159 1160 +1
|
src/packages.jl
Outdated
global DOWNLOAD_HELPER[] = Downloads.Downloader(; grace=0.1) | ||
|
||
# overwrite PKGMAN_DownloadURL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you verify that PackageManager still uses Julia's download feature? I am asking because the weird error about "could not import Downloads" suggests to me that perhaps JuliaImportPackage("Downloads")
returns false
and so the PackageManager code is not being loaded...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was also my understanding:
Whenever the warning occurs, the overwrite inside PackageManager does not happen, but up to now, we were anyhow overwriting the function afterwards, thus the code in PackageManager was actually never used.
With gap-packages/PackageManager/pull/134, the problem with the warning should disappear. It depends on the version of PackageManager used whether we can safely remove the code from GAP.jl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy with this but I'd suggest we wait until after PR #1029 is in, just to make sure we don't delay that one further.
call_with_catch
, and a few other improvements
call_with_catch
, and a few other improvementsCallJuliaFunctionWithCatch
, and a few other improvements
(motivated mainly by gap-packages/PackageManager/pull/134 and the issues mentioned there)
support keyword arguments in
CallJuliaFunctionWithCatch
do not use the deprecated and undocumented
Core._apply
update the initializations in
init_packagemanager
: ReplacingPKGMAN_DownloadURL
is no longer needed, replacingPKGMAN_RemoveDir
was in a wrong place.start better documentation of Julia syntax features