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

restyle API Ref #2102

Closed
wants to merge 62 commits into from
Closed

restyle API Ref #2102

wants to merge 62 commits into from

Conversation

piskvorky
Copy link
Owner

@piskvorky piskvorky commented Jun 25, 2018

Render a nicer API reference page. Continues from #1809.

I had trouble merging develop into this PR -- there were files with trailing whitespace (*_inner.c), and even two files with Windows line endings: lsi_dispatcher.py, lsi_worker.py, which prevented me from committing the resolved conflicts.

I solved the conflicts by removing trailing whitespace, converting line endings to Unix and then finishing the merge commit.

I also fixed the content of the documentation of the distributed LSI and LDA files, since it used incorrect docstring style and introduced some minor documentation bugs, in PR #1912 (specifically commit 1611f3a).

@piskvorky
Copy link
Owner Author

piskvorky commented Jun 25, 2018

@anotherbugmaster I'm getting some CircleCI errors after the merge, not sure how to resolve those. Can you please have a look?

@anotherbugmaster
Copy link
Contributor

@piskvorky, I'm on a vacation til 5th of July, but I'll have a look tonight

@anotherbugmaster
Copy link
Contributor

image

Fixed.

Removed *.rst with inner/outer d2v and w2v for now (not sure where to place them in the new API reference).

@piskvorky
Copy link
Owner Author

piskvorky commented Jun 27, 2018

Strange, I see some more merge conflicts, although I'm pretty sure I merged develop. And CircleCI still failing. @menshikh-iv do you know what's going on?

@menshikh-iv
Copy link
Contributor

menshikh-iv commented Jun 27, 2018

@piskvorky in #1809 no more merge conflicts and Circle build successfully, probably your merge was incorrect, let's work in #1809 and close current PR.
@anotherbugmaster please fix PEP8 issue in #1809 too

@piskvorky
Copy link
Owner Author

piskvorky commented Jun 27, 2018

I resolved the merge conflict. It was due to the recently merged non-API PR, which changed some files that this PR deleted.

We cannot just close this PR, it contains multiple important fixes and conflict resolutions in its merge commits. I suggest closing #1809 (I see some regressions in the recent commits there) and continuing here.

@@ -301,10 +301,12 @@ def __init__(self, corpus=None, id2word=None, dictionary=None, wlocal=utils.iden
Only when `pivot` is not None will pivoted document length normalization be applied.
Otherwise, regular TfIdf is used.
slope : float, optional
Parameter required by pivoted document length normalization which determines the slope to which
the `old normalization` can be tilted. This parameter only works when `pivot` is defined.
It is the parameter required by pivoted document length normalization which determines the slope to which
Copy link
Owner Author

Choose a reason for hiding this comment

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

Regression (PR contained had a newer, fixed version).

Out of curiosity -- what was this conflict from? This branch was already up-to-date with develop.

@menshikh-iv
Copy link
Contributor

This PR is important, but we need to fix current issues first, CC @anotherbugmaster

  1. "old apiref" doesn't looks like current apiref (3.4.0), that's a bug
  2. Many useful function & classes doesn't mention in new apiref
  3. General "linkage" issue (before we have one rst for module, now - rst for class/function, but not for module -> missing great module docstrings + general layout, this approach is OK for numpy, but bad for Gensim).

@mpenkov mpenkov self-assigned this Jan 24, 2019
@mpenkov mpenkov added the documentation Current issue related to documentation label Sep 29, 2019
@mpenkov mpenkov added the stale Waiting for author to complete contribution, no recent effort label Jun 10, 2020
@piskvorky
Copy link
Owner Author

piskvorky commented Feb 19, 2022

Apparently this PR contains some doc fixes, now lost in a sea of weird merges and abandoned code. I no longer recall what all this was about.

Is it worth fishing out those fixes and applying them to the current develop? Probably too much work, context lost, closing.

@piskvorky piskvorky closed this Feb 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Current issue related to documentation stale Waiting for author to complete contribution, no recent effort
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants