-
Notifications
You must be signed in to change notification settings - Fork 19
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
1st shot at suppressing the Polymake banner in Oscar #247
Conversation
src/Polymake.jl
Outdated
@@ -112,5 +115,8 @@ include("polynomial.jl") | |||
|
|||
include("polymake_direct_calls.jl") | |||
|
|||
include("generate_applications.jl") | |||
Base.CoreLogging.with_logger(Base.CoreLogging.NullLogger()) do |
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'd like a similar check here;
I like to see those when I'm using Polymake
, but maybe I'm alone? @saschatimme, @benlorenz?
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 don't really care whether this is shown.
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.
ok, let's get rid of this logging
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.
@fieker instead of this, feel free to remove
Polymake.jl/src/generate_applications.jl
Line 14 in fd04c00
@info "Generating module $mod" |
Isn't half this pull request already merged with #241? |
On Tue, Apr 07, 2020 at 05:29:03AM -0700, kalmarek wrote:
@kalmarek requested changes on this pull request.
> @@ -55,9 +55,12 @@ function __init__()
prepare_env()
end
+ show_banner = isinteractive() &&
Since we're already changing this why don't we use `parentmodule`?
in general have a look at https://github.com/JuliaLang/julia/blob/master/base/reflection.jl;
No preference
But how do you use parentmodule?
In init: parentmodule(Polymake) = Polymake
which is not helpful
`Base.package_locks` which doesn't do what we want
> @@ -112,5 +115,8 @@ include("polynomial.jl")
include("polymake_direct_calls.jl")
-include("generate_applications.jl")
+Base.CoreLogging.with_logger(Base.CoreLogging.NullLogger()) do
I'd like a similar check here;
I like to see those when I'm `using Polymake`, but maybe I'm alone? @saschatimme, @benlorenz?
Yep, I'd agree to that, but it is outside init, so I don't know if I can
detect Oscar at this point.
My experiments so far failed to transport the show_banner information to
the outside. Possibly since this is not in init...
As to the value of the information:
great if you know polymake
pointless for me... all this means: hey, I've triggered a polymake
function
However, core problem: I don't see how to do this.
Ideas are welcome!
…
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#247 (review)
|
On Tue, Apr 07, 2020 at 05:30:47AM -0700, Sascha Timme wrote:
Isn't half this pull request already merged with #241?
Sorry - aparently yes, but I hadn't updated mu installation
…
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#247 (comment)
|
@fieker use |
On Tue, Apr 07, 2020 at 06:03:54AM -0700, kalmarek wrote:
@fieker use `p = parentmodule(@__MODULE__)` and check if the last module of `p` is Oscar
Output
julia> using Oscar
[ Info: Precompiling Oscar [f1435218-dba5-11e9-1e4d-f1a5fab5fc13]
p = Polymake
p = Polymake
function __init__()
@initcxx
if using_binary
check_deps()
prepare_env()
end
try
show_banner = isinteractive() &&
!any(x->x.name in ["Oscar"], keys(Base.package_locks))
p = parentmodule(@__MODULE__)
@show p
… --
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#247 (comment)
|
indeed, each package is its own root nowadays |
have a look at oscar-system/Oscar.jl#90 const __weareoscar = haskey(ENV, "WE'RE_OSCAR_NO_BANNERS_PLEASE!") this checks essentially if a package has been loaded after from within the banner check becomes show_banner = isinteractive() && !__weareoscar which can be repeated actually across all cornerstone packages |
On Tue, Apr 14, 2020 at 08:51:37AM -0700, kalmarek wrote:
have a look at oscar-system/Oscar.jl#90
then we can write (before `__init__()`)
```julia
const __weareoscar = haskey(Base.ENV, "WE'RE_OSCAR_NO_BANNERS_PLEASE!")
```
this checks essentially if a package has been loaded after from within `Oscar`, and not if Oscar is installed somewhere on the system.
the banner check becomes
```julia
show_banner = isinteractive() && !__weareoscar
```
which can be repeated actually across all cornerstone packages
@fieker
How is the variable set?
… --
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#247 (comment)
|
it's set by Oscar, please have a look at oscar-system/Oscar.jl#90 |
On Tue, Apr 14, 2020 at 09:33:22AM -0700, kalmarek wrote:
it's set by Oscar, please have a look at oscar-system/Oscar.jl#90
I missed that part sorry.
But again: what problem is guranteed to be solved this way that the
current "hack" is not delivering?
Is it clear that the set is always going to happen before the
dependencies are loaded?
… --
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#247 (comment)
|
On Wed, Apr 15, 2020 at 12:08:19AM -0700, kalmarek wrote:
@kalmarek commented on this pull request.
> @@ -112,5 +115,8 @@ include("polynomial.jl")
include("polymake_direct_calls.jl")
-include("generate_applications.jl")
+Base.CoreLogging.with_logger(Base.CoreLogging.NullLogger()) do
@fieker instead of this, feel free to remove https://github.com/oscar-system/Polymake.jl/blob/fd04c00ecc27a282666acb07aeed32227d0b9914/src/generate_applications.jl#L14
Much easier!
… --
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#247 (comment)
|
@fieker I took my liberty to commit to your branch (actually surprised that I could), hope it's ok with you. The solution here uses Tests: _
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.4.0 (2020-03-21)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
|__/ |
julia> using Polymake
[ Info: Precompiling Polymake [d720cf60-89b5-51f5-aff5-213f193123e7]
polymake version 4.0
Copyright (c) 1997-2020
Ewgenij Gawrilow, Michael Joswig, and the polymake team
Technische Universität Berlin, Germany
https://polymake.org
This is free software licensed under GPL; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. _
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.4.0 (2020-03-21)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
|__/ |
julia> using Oscar
[ Info: Precompiling Oscar [f1435218-dba5-11e9-1e4d-f1a5fab5fc13]
WARNING: import of Hecke.example into Oscar conflicts with an existing identifier; ignored.
----- ----- ----- - -----
| | | | | | | | | |
| | | | | | | |
| | ----- | | | |-----
| | | | |-----| | |
| | | | | | | | | |
----- ----- ----- - - - -
...combining (and extending) GAP, Hecke, Nemo, Polymake and Singular
Version not installed ...
... which comes with absolutely no warranty whatsoever
Type: '?Oscar' for more information
(c) 2019-2020 by The Oscar Development Team btw. Oscar version is |
On Wed, Apr 15, 2020 at 01:55:48PM -0700, kalmarek wrote:
@fieker I took my liberty to commit to your branch (actually surprised that I could);
the solution now is to use `__is_root_module()` which tries to detect if `Polymake` was directly imported by user; if so the banner is printed; otherwise (e.g. imported within `Oscar`, but not only there) it is suppressed.
Great - thanks. I think this should then potentially put into AA to have
a clean and uniform interface!
… Tests:
```julia
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.4.0 (2020-03-21)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
|__/ |
julia> using Polymake
[ Info: Precompiling Polymake [d720cf60-89b5-51f5-aff5-213f193123e7]
polymake version 4.0
Copyright (c) 1997-2020
Ewgenij Gawrilow, Michael Joswig, and the polymake team
Technische Universität Berlin, Germany
https://polymake.org
This is free software licensed under GPL; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
```
```julia
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.4.0 (2020-03-21)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
|__/ |
julia> using Oscar
[ Info: Precompiling Oscar [f1435218-dba5-11e9-1e4d-f1a5fab5fc13]
WARNING: import of Hecke.example into Oscar conflicts with an existing identifier; ignored.
----- ----- ----- - -----
| | | | | | | | | |
| | | | | | | |
| | ----- | | | |-----
| | | | |-----| | |
| | | | | | | | | |
----- ----- ----- - - - -
...combining (and extending) GAP, Hecke, Nemo, Polymake and Singular
Version not installed ...
... which comes with absolutely no warranty whatsoever
Type: '?Oscar' for more information
(c) 2019-2020 by The Oscar Development Team
```
btw. Oscar version is `not installed` :)
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#247 (comment)
|
On Thu, Apr 16, 2020 at 08:27:02AM +0200, Claus Fieker wrote:
On Wed, Apr 15, 2020 at 01:55:48PM -0700, kalmarek wrote:
> @fieker I took my liberty to commit to your branch (actually surprised that I could);
> the solution now is to use `__is_root_module()` which tries to detect if `Polymake` was directly imported by user; if so the banner is printed; otherwise (e.g. imported within `Oscar`, but not only there) it is suppressed.
Great - thanks. I think this should then potentially put into AA to have
a clean and uniform interface!
Bad news... its not actually working. Problem:
it will supress a banner whenever Polymake is not the top module.
This is fine in Oscar: it would suppress
Polymake: it will print
But it will suppress for anyone using Polymake as a dependency
So
module MyModule
using Polymake
end
will not print the banner...
I've tried to test for the existence of Oscar, but
- Main.Oscar does not exist until after Polymake is compiled
- Base.PkgId("Oscar") will not create a full, correct PkgId
- I can get it to kind of work:
__is_root_module(m=Base.PkgId(@__MODULE__)) = haskey(Base.package_locks, m)
show_banner = isinteractive() && !__is_root_module(Base.PkgId(Base.UUID("f1435218-dba5-11e9-1e4d-f1a5fab5fc13"), "Oscar"))
but am not sure this is too much of an improvement.
There are a couple of features (yet) missing from the Julia packages...
…
> Tests:
> ```julia
> _
> _ _ _(_)_ | Documentation: https://docs.julialang.org
> (_) | (_) (_) |
> _ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
> | | | | | | |/ _` | |
> | | |_| | | | (_| | | Version 1.4.0 (2020-03-21)
> _/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
> |__/ |
>
> julia> using Polymake
> [ Info: Precompiling Polymake [d720cf60-89b5-51f5-aff5-213f193123e7]
> polymake version 4.0
> Copyright (c) 1997-2020
> Ewgenij Gawrilow, Michael Joswig, and the polymake team
> Technische Universität Berlin, Germany
> https://polymake.org
>
> This is free software licensed under GPL; see the source for copying conditions.
> There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> ```
> ```julia
> _
> _ _ _(_)_ | Documentation: https://docs.julialang.org
> (_) | (_) (_) |
> _ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
> | | | | | | |/ _` | |
> | | |_| | | | (_| | | Version 1.4.0 (2020-03-21)
> _/ |\__'_|_|_|\__'_| | Official https://julialang.org/ release
> |__/ |
>
> julia> using Oscar
> [ Info: Precompiling Oscar [f1435218-dba5-11e9-1e4d-f1a5fab5fc13]
> WARNING: import of Hecke.example into Oscar conflicts with an existing identifier; ignored.
> ----- ----- ----- - -----
> | | | | | | | | | |
> | | | | | | | |
> | | ----- | | | |-----
> | | | | |-----| | |
> | | | | | | | | | |
> ----- ----- ----- - - - -
>
> ...combining (and extending) GAP, Hecke, Nemo, Polymake and Singular
> Version not installed ...
> ... which comes with absolutely no warranty whatsoever
> Type: '?Oscar' for more information
> (c) 2019-2020 by The Oscar Development Team
> ```
>
> btw. Oscar version is `not installed` :)
>
> --
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub:
> #247 (comment)
|
that is not a bug, that is a feature feel free to alter it the way you wish |
I think this can be closed as it is superseded by #480. |
No description provided.