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

[DOC] doctest MEGA-issue #3925

Open
hmacdope opened this issue Nov 16, 2022 · 38 comments · Fixed by #4366, #4370, #4374 or #4521
Open

[DOC] doctest MEGA-issue #3925

hmacdope opened this issue Nov 16, 2022 · 38 comments · Fixed by #4366, #4370, #4374 or #4521

Comments

@hmacdope
Copy link
Member

Expected behavior

All the code in our docstring examples should be self-contained and run on its own. They should:

  • import the correct modules
  • demonstrate the desired behaviour
  • run correctly

Actual behavior

A lot of the examples in our docs do not run correctly as they are missing imports, steps, some code logic or all of the above.

By my initial count there are ~350 failing examples and only a handful that work.

We would love this to be fixed! This is a great opportunity for community members and budding open source contributors to get involved as there are lots of docstrings to tackle.

Luckily there is a way to test which doc examples are failing programatically!

  1. Navigate to the /package/doc/sphinx directory
  2. run make doctest

This will indicate which doc examples are failing.

If you want to tackle multiple doc changes at once editing your makefile command on L130 to include --keep-going will allow you to see the full set of errors.

eg

#L130
	$(SPHINXBUILD) -b doctest $(ALLSPHINXOPTS) $(BUILDDIR)/doctest --keep-going

Please contribute your changes in a new PR!

If you are new to making PRs and github in general, try our handy contributing guide! Feel free to ping our team if unsure. :)

@richardjgowers
Copy link
Member

Another idea would be to add the doctest in the CI and allow it to fail for now, then once we're passing make it another row in the CI.

@sahilempire
Copy link

sahilempire commented Dec 3, 2022

Hey maintainers, I want to work on this issue but I don't have enough knowledge, so can you please assign me and guide me through it?

@hmacdope
Copy link
Member Author

Hi @sahilempire sorry for the slow response. To address this issue you will need to run doctests that is navigate to package/doc/sphinx and run make doctest.

This will show you a bunch of failing doctests which we would like to fix up. Submit a PR fixing one or more! See our contributing guide for more info.

@chrispfae
Copy link
Contributor

Hi, I would also like to work on this issue. @sahilempire Have you already resolved some of the errors? Maybe we can split the work as there are a lot of failing examples?

@sahilempire
Copy link

@chrispfae I haven't done anything in this yet and if you want to do something then you can do it.

@IAlibay
Copy link
Member

IAlibay commented Jan 18, 2023

The point of this issue is that there are lots of things that need addressing and so it is assumed that folks might only tackle a smaller section of the failures. Please do raise PRs with fixes - we generally go on a "first PR basis", but we do appreciate it if folks want to coordinate so as to not duplicate efforts.

@nadeem-git-coder
Copy link

nadeem-git-coder commented Jan 26, 2023

Hello @hmacdope , I would like to contribute on this issue . Please assign me and guide throughout period.

@Tanmay-Malviya
Copy link

Respected Sir/madam, I am a 1 year experienced programmer, please let me work on this issue.

IAlibay pushed a commit that referenced this issue Jan 31, 2023
Towards #3925 
* Fix doctest for MDAnalysis.analysis.rms (#3925)
* Add name to AUTHORS and CHANGELOG
@chandra20500
Copy link

Hi I want to solve this issue. could you please assign this to me

orbeckst pushed a commit that referenced this issue Feb 13, 2023
* Adds missing imports in docstrings in MDAnalysis.core.topology
* solves failing darker_lint
* Part of #3925 for MDAnalysis.core.topology

Co-authored-by: Pratham Chauhan <Prathamchauhan2002@gmail.com>
@orbeckst
Copy link
Member

orbeckst commented Feb 13, 2023

To everyone who wants to work on this issue: that's great. But please note that we do not assign issues. Instead you should do the work and submit a PR that references this issue.

Normally, we only look at the first PR for each issue (see our GSOC FAQ Can I claim an issue to work on?) but for this umbrella issue we will look at any PR that addresses non-overlapping fixes.

When you start working on one doc test, check any linked, open PRs so that you only start work on an issue that has not been addressed yet.

@orbeckst
Copy link
Member

You can also look at merged linked PRs to see how to do it.

SorenKyhl added a commit to SorenKyhl/mdanalysis that referenced this issue Feb 23, 2023
resolves one of many inconsistencies in doctests
as described in issue MDAnalysis#3925
orbeckst pushed a commit that referenced this issue Feb 24, 2023
* Fix doctest for MDAnalysis.analysis.encore.similarity
* part of #3925
xhgchen added a commit to xhgchen/mdanalysis that referenced this issue Mar 7, 2023
* Towards MDAnalysis#3925
* Add imports to asunique() examples
* Fix code logic for ag*.ix in unique() and asunique():
change expected array to specify dtype=int64
* Fixes 102 failing tests (344 failures -> 242 failures)
@HeetVekariya
Copy link
Contributor

  • currently i am working on cluster.py files doctest error (it contains five errors, from which i have solved 4, but stuck at one).
    I am not able to solve this error, can you please guide me in this ?

code: cluster.py


Error:

UNEXPECTED EXCEPTION: AttributeError("'ClusterCollection' object has no attribute 'metadata'")
Traceback (most recent call last):
  File "/home/heet/anaconda3/envs/mdanalysis-dev/lib/python3.10/doctest.py", line 1350, in __run
    exec(compile(example.source, filename, "single",
  File "<doctest MDAnalysis.analysis.encore.clustering.cluster.cluster[10]>", line 1, in <module>
  File "<doctest MDAnalysis.analysis.encore.clustering.cluster.cluster[10]>", line 1, in <listcomp>
AttributeError: 'ClusterCollection' object has no attribute 'metadata'
/home/heet/Work/Development/mdanalysis/package/MDAnalysis/analysis/encore/clustering/cluster.py:145: UnexpectedException
===================================================== short test summary info =====================================================
FAILED analysis/encore/clustering/cluster.py::MDAnalysis.analysis.encore.clustering.cluster.cluster

@orbeckst
Copy link
Member

If you manually follow the example (eg in ipython or Jupyter) do you get the same error, i.e., the object misses the attribute? If yes, then look through the documentation to see if the attribute is documented and ought to exist. If it is documented but not there, raise an issue. If it’s not documented, remove the example.

@HeetVekariya
Copy link
Contributor

  • For me it is not possible to run it manually as it gives many internal import error.
  • Though i run below code to get the class type:
>>> print([type(cluster) for cluster in cluster_collection][:2])
[<class 'MDAnalysis.analysis.encore.clustering.ClusterCollection.ClusterCollection'>, 
<class 'MDAnalysis.analysis.encore.clustering.ClusterCollection.ClusterCollection'>]
  • Documentation says that there are two attributes associated with that class (MDAnalysis.analysis.encore.clustering.ClusterCollection.ClusterCollection) and that are elements and metadata, we can check here.
  • According to the docs this code must not give attribute error, but it does(shown below). So what should to be done now to solve it ?
>>> print([cluster.metadata["ensemble_membership"]
...          for cluster in cluster_collection][:2])
[array([1, 1, 1, 1, 2]), array([1, 1, 1, 1, 1])]

AttributeError: 'ClusterCollection' object has no attribute 'metadata'
  • I also noticed overflow in the docs here, please confirm if is it also on your side too or i am referring to the old version of the docs.

@orbeckst
Copy link
Member

orbeckst commented Jan 4, 2024

If the docs state that metadata should exist (as in https://docs.mdanalysis.org/2.7.0/documentation_pages/analysis/encore/clustering.html#MDAnalysis.analysis.encore.clustering.ClusterCollection.ClusterCollection ) and then tests raise an error then this looks like a bug — either in the code or in the docs. Please raise a separate issue for this specific case with all the necessary information from here. (The new issue should be self contained, i.e., copy over all necessary information and just put in a sentence along the lines of "As originally found in https://github.com/MDAnalysis/mdanalysis/issues/3925#issuecomment-1868292405 , ....".

@orbeckst
Copy link
Member

orbeckst commented Jan 4, 2024

I also noticed overflow in the docs here, please confirm if is it also on your side too or i am referring to the old version of the docs.

These are old docs (see the 2.0.0 in the URL and check the version menu in the lower left hand corner of the screen). 2.7.0 docs look fine to me.

@lunamorrow
Copy link

I am hoping to fix a doctest as part of my GSoC 2024 application and to get more familiar with the codebase, as I have only really utilised the analysis classes. I have set my cd to package/doc/sphinx, edited line 130 and attempted to run make doctest , but conf.py inside sphinx is unable to find the MDAnalysis module so is erroring out at this point before I can even see where the doctest issues are. I have double checked that my environment is correctly activated and python -c 'import MDAnalysis as mda; print(mda.__version__)' does print 2.8.0-dev0 to the console as expected. Any guidance so I can get stuck into fixing some code would be greatly appreciated. Thanks! @hmacdope

@hmacdope
Copy link
Member Author

@lunamorrow could you post the error log here?

Incidentally, while you can do a doctest problem for your GSOC PR. I would reccomend doing an issue that has you tackling a more coding than a docs problem, as more substantial and will show your skills better.

@lunamorrow
Copy link

lunamorrow commented Mar 22, 2024

(mdanalysis-dev) (base) lunamorrow@d-i89-211-193 sphinx % make doctest                                                
sphinx-build -v -W -b doctest   source ../doctest --keep-going
Running Sphinx v4.2.0

Traceback (most recent call last):
  File "/Users/lunamorrow/opt/anaconda3/lib/python3.9/site-packages/sphinx/config.py", line 328, in eval_config_file
    exec(code, namespace)
  File "/Users/lunamorrow/mdanalysis/package/doc/sphinx/source/conf.py", line 17, in <module>
    import MDAnalysis as mda
ModuleNotFoundError: No module named 'MDAnalysis'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/lunamorrow/opt/anaconda3/lib/python3.9/site-packages/sphinx/cmd/build.py", line 276, in build_main
    app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
  File "/Users/lunamorrow/opt/anaconda3/lib/python3.9/site-packages/sphinx/application.py", line 216, in __init__
    self.config = Config.read(self.confdir, confoverrides or {}, self.tags)
  File "/Users/lunamorrow/opt/anaconda3/lib/python3.9/site-packages/sphinx/config.py", line 172, in read
    namespace = eval_config_file(filename, tags)
  File "/Users/lunamorrow/opt/anaconda3/lib/python3.9/site-packages/sphinx/config.py", line 341, in eval_config_file
    raise ConfigError(msg % traceback.format_exc()) from exc
sphinx.errors.ConfigError: There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/Users/lunamorrow/opt/anaconda3/lib/python3.9/site-packages/sphinx/config.py", line 328, in eval_config_file
    exec(code, namespace)
  File "/Users/lunamorrow/mdanalysis/package/doc/sphinx/source/conf.py", line 17, in <module>
    import MDAnalysis as mda
ModuleNotFoundError: No module named 'MDAnalysis'


Configuration error:
There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/Users/lunamorrow/opt/anaconda3/lib/python3.9/site-packages/sphinx/config.py", line 328, in eval_config_file
    exec(code, namespace)
  File "/Users/lunamorrow/mdanalysis/package/doc/sphinx/source/conf.py", line 17, in <module>
    import MDAnalysis as mda
ModuleNotFoundError: No module named 'MDAnalysis'

make: *** [doctest] Error 2

Ok noted, thanks @hmacdope, I will look for another issue to tackle. I saw a few tagged with GSOC starter that did not already have a pull submitted, but it seems someone has already requested to do them for their GSOC PR and I did not want to double up as I understand contributions/pull requests are on a first submitted basis. I will have another look :)

orbeckst pushed a commit that referenced this issue Mar 29, 2024
* part of #3925 (doc test mega issue)
* Fixed all failing doctests in groups.py using sphinx directives
   NOTE: These tests will not pass using `pytest -v --disable-pytest-warnings --doctest-modules` 
   since I used sphinx directives to allow for concise example code. This can be changed but the 
   documentation will become more verbose.
* Updated CHANGELOG
@IAlibay IAlibay reopened this Mar 29, 2024
@orbeckst
Copy link
Member

Sorry for the auto-close! Thank you for noticing and re-opening @IAlibay !

orbeckst pushed a commit that referenced this issue Mar 29, 2024
* contributes to #3925
* fix analysis.align doctest errors:
  added :: in the end of two sentences, to remove two "not defined" errors
* Update AUTHORS
@orbeckst
Copy link
Member

Sorry, writing "partially fixes ISSUE" apparently auto-closes. Re-opening.

@orbeckst orbeckst reopened this Mar 29, 2024
@orbeckst
Copy link
Member

In order to make it easier to manage this big parent issue, could we ask contributors who want to do doctest fixing to do the follwing:

  1. raise a new issue describing the specific doc tests that you intend to fix; in the issue text mention "contributes to issue [DOC] doctest MEGA-issue #3925" so that the new issue is linked to the one here
  2. start a pull request that states that it fixes the newly created issue

In this way contributors learn to raise issues and work on an immediately well-defined problem, and we better keep track of what's open and closed, and we don't accidentally close this omnibus issue.

yuxuanzhuang pushed a commit that referenced this issue Apr 1, 2024
* contributes to #3925
* fix lammps.py doctest errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment