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

Fix some of the warnings/ deprecated functions #3080

Merged
merged 3 commits into from
Mar 22, 2021
Merged

Fix some of the warnings/ deprecated functions #3080

merged 3 commits into from
Mar 22, 2021

Conversation

FredHappyface
Copy link
Contributor

Closes #3066
Addresses #3014

Remaining (removed windows specific warnings):

.tox\py39-win\lib\site-packages\scipy\sparse\sparsetools.py:21
  c:\users\dell\documents\github\gensim\.tox\py39-win\lib\site-packages\scipy\sparse\sparsetools.py:21: DeprecationWarning: `parse.sparsetools` is deprecated!
  scipy.sparse.sparsetools is a private module for scipy.sparse, and should not be used.
    _deprecated()

gensim/test/test_api.py::TestApi::test_load_dataset
gensim/test/test_api.py::TestApi::test_multipart_load
  c:\users\dell\documents\github\gensim\.tox\py39-win\lib\site-packages\smart_open\smart_open_lib.py:479: DeprecationWarning:unction is deprecated.  See https://github.com/RaRe-Technologies/smart_open/blob/develop/MIGRATING_FROM_OLDER_VERSIONS.rst fo
information
    warnings.warn(message, category=DeprecationWarning)

gensim/test/test_ldaseqmodel.py::TestLdaSeq::test_doc_topic
gensim/test/test_ldaseqmodel.py::TestLdaSeq::test_dtype_backward_compatibility
gensim/test/test_ldaseqmodel.py::TestLdaSeq::test_topic_word
  C:\Users\Dell\Documents\GitHub\gensim\gensim\models\ldaseqmodel.py:297: RuntimeWarning: divide by zero encountered in doublrs
    convergence = np.fabs((bound - old_bound) / old_bound)

gensim/test/test_lsimodel.py::TestLsiModel::test_online_transform
gensim/test/test_lsimodel.py::TestLsiModel::test_online_transform
gensim/test/test_lsimodel.py::TestLsiModel::test_online_transform
  c:\users\dell\documents\github\gensim\.tox\py39-win\lib\site-packages\numpy\matrixlib\defmatrix.py:1109: PendingDeprecation: the matrix subclass is not the recommended way to represent matrices or deal with linear algebra (see https://docs.scipy.orumpy/user/numpy-for-matlab-users.html). Please adjust your code to use regular ndarray.
    return matrix(concatenate(arr_rows, axis=0))

gensim/test/test_miislita.py: 1 warning
gensim/test/test_sharded_corpus.py: 15 warnings
  C:\Users\Dell\Documents\GitHub\gensim\gensim\interfaces.py:92: UserWarning: corpus.save() stores only the (tiny) iteration 
in memory; to serialize the actual corpus content, use e.g. MmCorpus.serialize(corpus)
    warnings.warn(

@@ -93,7 +93,8 @@

"""

import collections

import collections.abc as collections
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any real benefit in doing this instead of:

Suggested change
import collections.abc as collections
import collections.abc

You're saving 4 characters, but making it less obvious what's actually being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of keeping small diffs at the time but I agree that it would make more sense to change it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is a somewhat valid reason, but think about the trade-off:

  1. We do a bit more work during this review (one-off cost)
  2. People reading the code may be confused (repetitive cost)

Better to pay once up front and save future generations of gensim developers the pain (I'm exaggerating, obviously, but you probably get my point).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah absolutely! I'll get that changed. Hopefully I'll have it sorted in a couple hours

@mpenkov
Copy link
Collaborator

mpenkov commented Mar 22, 2021

OK, merging. Thank you for your work @FredHappyface !

@mpenkov mpenkov merged commit 04f3414 into piskvorky:develop Mar 22, 2021
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.

Warnings of deprecated functions
2 participants