The qqbar.py and expression.pyx changes do not belong into this pull request.

I am not sure about the deprecation. I think that it is required that old code works for about one year, and then the deprecated functionality is removed.

I don't know how to best do it, but one way is to introduce an optional keyword like monomials_as_vector=False, the deprecation is triggered only if we have monomials_as_vector == False, but in this case a matrix is returned (as before). If, however, monomials_as_vector == True, then a vector is returned and no warning is triggered.

The downside is that both seeing the deprecation warning but also always having to provide the keyword is extremely annoying to users. See #36073. Thus, if we follow this model, we have to think hard about a good name for the keyword argument.

How about below implementation, I think we should not show deprecation message when user is using vector_form(I don't whether this name works or not but for now I am using this for reference) attribute have some value then we should not show deprecation message, so what if we make vector_form by default None, and do below changes ?

        if vector_form == True:
            return A, vector(m)
        else:
            if vector_form == None:
                deprecation(37027,'the function PolynomialSequence.coeffient_matrix() will'
                                  ' return a vector of monomilas instead of a matrix as second element in tuple in the future,'
                                  ' please use vector_form argument to get desired type.')
            return A, Matrix(Matrix(R,nm,1,m))

But still I am not sure whether this is a good idea or not? And also don't know wether the name for the attribute is good or not.

To make the linter happy, you also need to remove trailing whitespace.

After that, I think we should ask on sage-devel whether this is the way to go. One problem I see (and do not know how to solve) is that, eventually, we want to remove the vector_form keyword argument again. It would be nice if we could do this in one deprecation. Do you want to ask on sage-devel, or should I do it?

Ok, I'll ask. And thank you for your help !

Hi @RuchitJagodara ! I am now a bit confused. Could you let me now what the state of the PR is now? Was the problem with for c, m in poly not a problem after all?

You squashed the commits, right?

Hi @RuchitJagodara ! I am now a bit confused. Could you let me now what the state of the PR is now? Was the problem with for c, m in poly not a problem after all?

No, there was no problem with that.

You squashed the commits, right?

Yes.

I am really sorry for the confusion caused by this change. I reset the branch because there were some conflicts in my branches due to which my one PR got merged without any positive review, so I was just sorting out that issue. Therefore, I had to reset this branch, but now I have included all commits that were missing in this branch. I am really sorry for the inconvenience caused because of this. 😞

Hi @RuchitJagodara ! I am now a bit confused. Could you let me now what the state of the PR is now? Was the problem with for c, m in poly not a problem after all?

No, there was no problem with that.

Oh, that's good.

I am really sorry for the confusion caused by this change. I reset the branch because there were some conflicts in my branches due to which my one PR got merged without any positive review, so I was just sorting out that issue. Therefore, I had to reset this branch, but now I have included all commits that were missing in this branch. I am really sorry for the inconvenience caused because of this. 😞

Absolutely no problem at all! Could you just do the minor modifications? I would then wait for a green bot and set it to "positive review".

I am so glad that you did this, by the way!

Does this PR need any further changes becuase I have done all necessary changes.

And should I add some tests for new function becuase codecov/patch check is failing ?

@RuchitJagodara, I think there is a misunderstanding: there are still comments which have not been adressed, most importantly the question about line 771 we discussed before. The last answer I saw I responded to in comment #37035 (comment), but then the code changed back.

I have addreseed that in comment.

Actually, there is a problem with for c, m in poly:, because this line will give an error when there is a constant term appearing in the polynomial.

When you asked Was the problem with for c, m in poly not a problem after all?, I thought you were asking that if that was the cause of the change. So I just replied no on that. But ya that code won't work for some cases but I have modified it accordingly and it should work now.

Actually, there is a problem with for c, m in poly:, because this line will give an error when there is a constant term appearing in the polynomial.

Could you please give me a simple example for that? If I do

sage: P.<x,y> = QQ[]
sage: p = x^2*y + 1
sage: [(c, m) for c, m in p]
[(1, x^2*y), (1, 1)]

it works.

Hmm... It is working fine now. I think something should have changed because the last time I tried this method, it was not working; it was giving me a ValueError, saying there were not enough values to unpack. Because of that, I changed the code at that time after leaving a comment. However, I think now it is safe to go this way because I have checked the code in different test cases, and it is working fine.

@mantepse, I don't know why, but the checks are not running; they are queued. Do you know what the problem is?

That's indeed very confusing.

Anyhow, could you please also do the 2 whitespace comments (which you can see on "Files changed" tab)?

@mantepse, I don't know why, but the checks are not running; they are queued. Do you know what the problem is?

I think that's just because too many people are testing right now.

(So it really would be good to do the final changes now, in order not to waste resources)

Anyhow, could you please also do the 2 whitespace comments (which you can see on "Files changed" tab)?

Are you talking about these lines ?

        Return the matrix of coefficients ``A`` and
        the matching vector of monomials ``v``, such that ``A*v == vector(self)``.

        Thus value of ``A[i,j]`` corresponds the coefficient of the
        monomial ``v[j]`` in the ``i``-th polynomial in this system.

        Monomials are ordered w.r.t. the term ordering of ``order``
        if given; otherwise, they are ordered w.r.t. ``self.ring()``
        in reverse order, i.e., such that the smallest entry comes last.

No, I'm talking about
very minor: R is now used only once (in line 768), so you could inline it.
and
extremely minor: change the whitespace as follows: ...

Can you see these comments? github is quite tricky at times...

Can you see these comments? github is quite tricky at times...

I am not able to see those whitespace comments but I have removed the R from the code.

Here is a screenshot, you should see this in the "Files changed" tab.

Oh, I see. I am not able to see this comments. Could you please submit the review, I think after submitting the review I will be able to see those comments. Because it is showing pending tag on the comments.

Although, I have updated the code with suggested changes, please let me know if there is anything else.

Oh, I see. I am not able to see this comments. Could you please submit the review, I think after submitting the review I will be able to see those comments. Because it is showing pending tag on the comments.

No, the pending tag means that the code was not yet changed. It would be really important that you can see these comments for developing sage

Hmmm.... It's very strange because I am not able to see those comments.

And the resolved conversation that you can see in the above screenshot is review

Dear @RuchitJagodara, I now see the problem: the BooleanPolynomialRing is a polynomial ring which does not adhere to the polynomial protocol 👎 Here is what I would suggest to do:

diff --git a/src/sage/rings/polynomial/multi_polynomial_sequence.py b/src/sage/rings/polynomial/multi_polynomial_sequence.py
index 72ce72f748..9519b2d0c9 100644
--- a/src/sage/rings/polynomial/multi_polynomial_sequence.py
+++ b/src/sage/rings/polynomial/multi_polynomial_sequence.py
@@ -750,7 +750,6 @@ class PolynomialSequence_generic(Sequence_generic):
         from sage.modules.free_module_element import vector
         from sage.matrix.constructor import Matrix
 
-        f = tuple(self)
         if order is None:
             v = sorted(self.monomials(), reverse=True)
         else:
@@ -760,8 +759,8 @@ class PolynomialSequence_generic(Sequence_generic):
                 raise ValueError("order argument can only accept list or tuple")
 
         y = dict(zip(v, range(len(v))))  # construct dictionary for fast lookups
-        A = Matrix(self.ring().base_ring(), len(f), len(v), sparse=sparse)
-        for x, poly in enumerate(f):
+        A = Matrix(self.ring().base_ring(), len(self), len(v), sparse=sparse)
+        for x, poly in enumerate(self):
             for c, m in poly:
                 try:
                     A[x, y[m]] = c
@@ -1695,6 +1694,60 @@ class PolynomialSequence_gf2(PolynomialSequence_generic):
         else:
             return PolynomialSequence_generic.reduced(self)
 
+    def coefficients_monomials(self, order=None, sparse=True):
+        """
+        Return the matrix of coefficients ``A`` and
+        the matching vector of monomials ``v``, such that ``A*v == vector(self)``.
+
+        Thus value of ``A[i,j]`` corresponds the coefficient of the
+        monomial ``v[j]`` in the ``i``-th polynomial in this system.
+
+        Monomials are ordered w.r.t. the term ordering of ``order``
+        if given; otherwise, they are ordered w.r.t. ``self.ring()``
+        in reverse order, i.e., such that the smallest entry comes last.
+
+        INPUT:
+
+        - ``sparse`` - construct a sparse matrix (default: ``True``)
+        - ``order`` - a list or tuple specifying the order of monomials (default: ``None``)
+
+        EXAMPLES::
+
+            sage: # needs sage.rings.polynomial.pbori
+            sage: B.<x,y,z> = BooleanPolynomialRing()
+            sage: F = Sequence([x*y + y + 1, z + 1])
+            sage: A, v = F.coefficients_monomials()
+            sage: A
+            [1 1 0 1]
+            [0 0 1 1]
+            sage: v
+            (x*y, y, z, 1)
+            sage: A*v
+            (x*y + y + 1, z + 1)
+        """
+        from sage.modules.free_module_element import vector
+        from sage.matrix.constructor import Matrix
+
+        if order is None:
+            v = sorted(self.monomials(), reverse=True)
+        else:
+            if isinstance(order, (list, tuple)):
+                v = order
+            else:
+                raise ValueError("order argument can only accept list or tuple")
+
+        R = self.ring().base_ring()
+        one = R.one()
+        y = dict(zip(v, range(len(v))))  # construct dictionary for fast lookups
+        A = Matrix(R, len(self), len(v), sparse=sparse)
+        for x, poly in enumerate(self):
+            for m in poly:
+                try:
+                    A[x, y[m]] = one
+                except KeyError:
+                    raise ValueError("order argument does not contain all monomials")
+        return A, vector(v)
+
 
 class PolynomialSequence_gf2e(PolynomialSequence_generic):
     r"""

I have updated the code according to your suggestions.

Documentation preview for this PR (built with commit 77b4f7c; changes) is ready! 🎉

Now, I am able to see those comments to which you were referring earlier.

If you have time, this would be an excellent next project, together with #37075.

I think that issue has been resolved, but it seems that there is a function coeffcient_matrix in toy_variety.py. In which I found a note...

    .. NOTE::

        This function may be merged with
        :meth:`sage.rings.polynomial.multi_polynomial_sequence.PolynomialSequence_generic.coefficient_matrix()` in the future.

So, should I merge this function with this coefficients_monomials function because this note was added two years ago, so I think it is okay if we merge this. What do you say, @mantepse?