Skip to content

Make psutil an optional dependency#4634

Merged
gvanrossum merged 8 commits intopython:masterfrom
miedzinski:psutil-extra
Feb 26, 2018
Merged

Make psutil an optional dependency#4634
gvanrossum merged 8 commits intopython:masterfrom
miedzinski:psutil-extra

Conversation

@miedzinski
Copy link
Contributor

@miedzinski miedzinski commented Feb 25, 2018 •
edited
Loading

  • in case of psutil not installed, daemon reports it and suggests installing it
  • on Windows it isn't declared as dependency nor reported as missing

Fixes #4593.

@miedzinski
Copy link
Contributor Author

miedzinski commented Feb 25, 2018

Perhaps a better extra name could be used? I've picked dmypy, because psutil is used there.

emmatyping
emmatyping requested changes Feb 25, 2018
View reviewed changes
Copy link
Member

@emmatyping emmatyping left a comment

Choose a reason for hiding this comment

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

Looks like a good start, but this should be documented (maybe in the installation section?). Also, I think there should be an error message here

mypy/mypy/dmypy_server.py

Line 363 in 0c12b21

try:
if it tries to import psutil and fails. Maybe psutil not found, run pip install mypy[dmypy] to install the needed components for dmypy?

setup.py Outdated
],
extras_require = {
':python_version < "3.5"': 'typing >= 3.5.3',
'dmypy': 'psutil >= 5.4.0, < 5.5.0',
Copy link
Member

@emmatyping emmatyping Feb 25, 2018

Choose a reason for hiding this comment

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

You should add :sys_platform!="win32" after dmypy, since it is not used on Windows.

@miedzinski
Copy link
Contributor Author

miedzinski commented Feb 25, 2018 •
edited
Loading

I agree that docs should be updated, but I don't know what dmypy actually is. All I could do is to mention an extra dependency, but without any explanation about its purpose. And dmypy isn't documented at all anyway.

@miedzinski
Copy link
Contributor Author

miedzinski commented Feb 25, 2018

Oh and I'm pretty sure that using print there is wrong, because I don't see any output when running dmypy. Is there any logger I can't find?

@gvanrossum
Copy link
Member

gvanrossum commented Feb 25, 2018

Can you point me to the docs that explain this notation works? Both the 'dmypy': prefix in the extras_require list and the pip install mypy[dmypy] form are new to me.

I think the best way to surface the error is to stick it in the res dictionary, e.g. with the key 'memory_psutil_missing'.

@miedzinski
Copy link
Contributor Author

miedzinski commented Feb 25, 2018

@gvanrossum
https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies
https://packaging.python.org/tutorials/installing-packages/#installing-setuptools-extras
https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-platform-specific-dependencies

@gvanrossum
Copy link
Member

gvanrossum commented Feb 25, 2018

Assuming the tests pass I'm fine with it.

@emmatyping
Copy link
Member

emmatyping commented Feb 25, 2018

I restarted the 3.6 build on Travis as the stubgen tests seem to be flaking.

@miedzinski
Copy link
Contributor Author

miedzinski commented Feb 26, 2018

I've updated PR description in such way that it describes all changes to the codebase.

JukkaL
JukkaL requested changes Feb 26, 2018
View reviewed changes
docs/source/getting_started.rst Outdated

$ python3 -m pip install mypy

Optionally you can install extra dependencies for daemon server with:
Copy link
Collaborator

@JukkaL JukkaL Feb 26, 2018

Choose a reason for hiding this comment

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

Please remove this since dmypy is otherwise undocumented (and unsupported). This will only confuse users.

@gvanrossum gvanrossum merged commit 55dadfc into python:master Feb 26, 2018
@gvanrossum
Copy link
Member

gvanrossum commented Feb 26, 2018

Thanks!

carljm added a commit to carljm/mypy that referenced this pull request Feb 28, 2018
@carljm
Merge branch 'master' into import-cycle
2a37e80 
* master: (27 commits)
  Don't call --strict-optional and --incremental experimental (python#4642)
  Sync typeshed (python#4641)
  Fix callable types with inconsistent argument counts (python#4611)
  Fix example (add 'class A:')
  Make psutil an optional dependency (python#4634)
  mypy and mypy_extensions aren't posix only (python#3765)
  Documentation for attr support (python#4632)
  Use read_with_python_encoding in stubgen to handle file encoding (python#3790)
  Sync typeshed (python#4631)
  Add remaining core team emails to CREDITS (python#4629)
  Fix issues with attr code. (python#4628)
  Better support for converter in attrs plugin. (python#4607)
  Clean up credits (python#4626)
  Support type aliases in fine-grained incremental mode (python#4525)
  Fine-grained: Fix crash caused by unreachable class (python#4613)
  Treat divmod like a binary operator (python#4585)
  Sync typeshed (python#4605)
  Fine-grained: Don't infer partial types from multiple targets (python#4553)
  Fine-grained: Compare symbol table snapshots when following dependencies (python#4598)
  Fix type of forward reference to a decorated class method (python#4486)
  ...
yedpodtrzitko pushed a commit to kiwicom/mypy that referenced this pull request Mar 15, 2018
@miedzinski @yedpodtrzitko
Make psutil an optional dependency (python#4634)
fa599b0 
If it's not installed, when running `dmypy status -v` a message will be displayed instead of the memory stats.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JukkaL JukkaL JukkaL approved these changes

@emmatyping emmatyping emmatyping approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@miedzinski @gvanrossum @emmatyping @JukkaL