Rename isweighted -> is_weighted
#2953
Conversation
| function is_weighted(P::ProjSpc) | ||
| return !all(x->isone(x[1]), P.Rx.d) |
There was a problem hiding this comment.
I find this method problematic -- why is a projective space in which all weights are 1 "not weighted" ?
src/Rings/orderings.jl
Outdated
| @@ -124,7 +124,7 @@ function _unique_var_indices(a::AbstractVector{<:MPolyRingElem}) | |||
| return z | |||
| end | |||
|
|
|||
| function isweighted(ord::Symbol) | |||
| function is_weighted(ord::Symbol) | |||
There was a problem hiding this comment.
I find this method problematic: in how far is a symbol weighted? This is perhaps OK for an internal helper, but not for something we export. I would suggest to call this _is_weighted
There was a problem hiding this comment.
I have the impression that this function should be removed completely. It is not used (apparently it was used originally for a show method) and monomial orderings should not be represented by symbols nowadays. So if the functionality is needed, there should be a is_weighted(::MonomialOrdering) in my opinion.
It looks like one could easily implement this with
is_weighted(::MonomialOrdering) = false
is_weighted(::WSymbOrdering) = true
but no guarantee.
There was a problem hiding this comment.
The reason why I opened this PR is that I need this for #2936. Users should be able to provide an ordering via symbol as an input there. So please don't remove
Codecov Report
@@ Coverage Diff @@
## master #2953 +/- ##
==========================================
- Coverage 80.38% 80.35% -0.04%
==========================================
Files 469 470 +1
Lines 66383 66654 +271
==========================================
+ Hits 53363 53557 +194
- Misses 13020 13097 +77
|
lgoettgens
changed the title
isweighted -> is_weighted and exportisweighted -> is_weighted
This is used in #2936, and I think it should follow our naming scheme. As the function was not exported before, I think we can omit the deprecation.