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

Subpackage refactor. Fix 1584 #1618

Closed
wants to merge 9 commits into from
Closed

Conversation

menshikh-iv
Copy link
Contributor

Fix for #1584.

@menshikh-iv
Copy link
Contributor Author

menshikh-iv commented Oct 10, 2017

What's done:

  • Removed gensim.examples

  • Removed gensim.nosy

  • Scripts:

    • Removed from gensim/scripts (because all of this is duplicates)
      • make_wiki_lemma.py
      • make_wiki_online.py
      • make_wiki_online_lemma.py,
      • make_wiki_online_nodebug.py
      • make_wiki.py
    • Rename gensim/scripts/make_wikicorpus.py -> gensim/scripts/make_wiki.py
    • Removed word2vec_standalone.py
  • Moved gensim.topic_coherence to gensim.models._coherence

  • Moved gensim.summarization to gensim.models.summarization

  • Moved gensim.utils to gensim.utils.utils (with imports in gensim/utils/__init__.py, no differences for users)

  • Moved with raneming gensim.parsing.* to gensim.utils.text_utils

  • Fixed doc structure (according to changes)

I re-create PR according to discussion from #1607 and #1584, please review again @piskvorky @macks22

@menshikh-iv menshikh-iv added the breaks backward-compatibility Change breaks backward compatibility label Oct 10, 2017
@piskvorky
Copy link
Owner

piskvorky commented Oct 10, 2017

Regarding the various make_wiki_*.py duplicates: these were originally symlinks. They became duplicates (regular files) only by error of a committer.

Removing them will break some functionality, because the make_wiki script behaves differently based on what filename it was run as.

These parameters are probably better served as proper CLI options, rather than encoded in the filename. Just saying that simply removing the "duplicates" will prevent users from accessing existing functionality.

@menshikh-iv
Copy link
Contributor Author

menshikh-iv commented Oct 10, 2017

@piskvorky
All different behavior implemented through this arg, that's very bad, why not concrete CLI option? I'll fix it.

@menshikh-iv
Copy link
Contributor Author

Close (because conflict is really big), I'll re-create it with additions when we plan to create major release

@menshikh-iv menshikh-iv deleted the subpackage-refactor-2 branch March 1, 2018 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks backward-compatibility Change breaks backward compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants