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

Update example_top_artists.rst #1662

Merged
merged 2 commits into from
Jun 17, 2016
Merged

Update example_top_artists.rst #1662

merged 2 commits into from
Jun 17, 2016

Conversation

mbelmadani
Copy link
Contributor

@mbelmadani mbelmadani commented Apr 20, 2016

Description

Improved documentation by suggesting to set the python path if an import error occurs.

Motivation and Context

Since this documentation is part of introductory material, it is probably a good idea to suggest this common mistake. The error this addresses has also been mentioned in a previous issue (#1321).

Have you tested this? If so, how?

Added example command to work around the class missing from PYTHONPATH issue which causes an import error when calling the luigi command line tool.

Added example command to work around the class missing from PYTHONPATH issue which causes an import error when calling the luigi command line tool.
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @jcrobak, @steenzout and @erikbern to be potential reviewers

@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 21, 2016

This looks a bit confusing. Are you sure you understand PYTHONPATH? I think the reason it works with PYTHONPATH='' is that when it's empty it defaults to including to check the current folder. Hence your second example is a bit confusing. Why both cd there and set the PYTHONPATH to that folder?

Other than that. I love that you took time to write these docs :)

Removed redundant example.
@alfonsomhc
Copy link
Contributor

It would be nice if this could be added to the documentation. I run into this problem when following the example in http://luigi.readthedocs.io/en/stable/command_line.html
This extra information would have saved me some time...

@Tarrasch
Copy link
Contributor

Ah, I missed that this PR was updated since my last comment. Thanks for bumping this thread @alfonsomhc. I'm merging, hoping that the next user will have a better experience. :)

@Tarrasch Tarrasch merged commit b871e9d into spotify:master Jun 17, 2016
@mbelmadani mbelmadani deleted the patch-1 branch June 20, 2016 22:08
p7k pushed a commit to Celmatix/luigi that referenced this pull request Jul 11, 2016
* spotify/master: (25 commits)
  Version 2.2.0
  Add tests for hashing parameters (spotify#1719)
  Update call to iteritems in luigi/tools/deps: deprecated in Python 3 (spotify#1749)
  Reset terminal colors in external_program (spotify#1742)
  Caches get_autoconfig_client on a per-thread basis
  Fix bug with GCSFlagTarget
  Add additional event handlers to tasks (spotify#1698)
  Reduce number of get_params calls in common_params.
  Removes redundant function definitions from rpc and server (spotify#1734)
  Fix salesforce default content type (spotify#1724)
  Rename MockTarget class variable _fn to path
  Remove MockTarget path property
  Deprecated LocalTarget fn propery
  Add note about underscore in parameter names (spotify#1729)
  Remove tracking url callback hack (spotify#1722)
  Consistent Luigi spelling in docs (spotify#1723)
  Update example_top_artists.rst (spotify#1662)
  Add combiner to docstrings in mrrunner
  Add luigi-deps-tree visualising tool (spotify#1680)
  Adding release step for Debian packages. (spotify#1718)
  ...
This was referenced Jun 29, 2022
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.

4 participants