Add docs for collection_items and fetch methods

[prcutler]

This PR:

• Documents the collection_items method introduced in PR#91.
• Adds a new chapter on how to use the fetch() method of an object, requested in issue Add docs to use .fetch  #103.
• Applies fixes and additions to the Collection Data chapter in general.

This should build correctly, but I'm getting an error trying to do make html when building in a new environment. Submitting the PR just in case someone does want to test and see if it builds.

I tested the methods and used the text written by AnssiAhola in the discussions to update the docs. Any feedback on the docs is welcome.

Paul

Here's my error in a 3.9 venv:

 make html                                                                                                                                                                                                                in zsh at 08:33:10
Traceback (most recent call last):
  File "/Users/prcutler/workspace/discogs_client/.venv/bin/sphinx-build", line 5, in <module>
    from sphinx.cmd.build import main
  File "/Users/prcutler/workspace/discogs_client/.venv/lib/python3.9/site-packages/sphinx/cmd/build.py", line 25, in <module>
    from sphinx.application import Sphinx
  File "/Users/prcutler/workspace/discogs_client/.venv/lib/python3.9/site-packages/sphinx/application.py", line 43, in <module>
    from sphinx.registry import SphinxComponentRegistry
  File "/Users/prcutler/workspace/discogs_client/.venv/lib/python3.9/site-packages/sphinx/registry.py", line 24, in <module>
    from sphinx.builders import Builder
  File "/Users/prcutler/workspace/discogs_client/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 26, in <module>
    from sphinx.util import import_object, logging, progress_message, rst, status_iterator
  File "/Users/prcutler/workspace/discogs_client/.venv/lib/python3.9/site-packages/sphinx/util/rst.py", line 21, in <module>
    from jinja2 import Environment, environmentfilter
ImportError: cannot import name 'environmentfilter' from 'jinja2' (/Users/prcutler/workspace/discogs_client/.venv/lib/python3.9/site-packages/jinja2/__init__.py)
make: *** [html] Error 1

[JOJ0]

The docs build fine in my "already existing" venv. No Jinja error.

I can confirm that I get an error (even different from yours) when installing in a fresh venv and following the docs.

I think we shouldn't worry too much about what exactly the error tells us but consider that it just points to some version/dependency issues.

I had a look into sphinx_requirements.txt. For some reason I hardcoded all the versions of the packages in there, probably just to get a "known to be working" configuration for docs building. This does not seem to work anymore. Maybe it's because now we just use a slightly different Python 3.9 version.

My suggestion now would be:

• Create a fresh env
• Install using the regular method of setup.py install/develop (I am not sure if requirements.txt is used or ignored this way. FIXME need to learn that -> Yes it's ignored, setup.py and requirements.txt don't interact, our setup.py install_requires list is much shorter than the package list in requirements.txt)
• Then install the additional packages we need for docs building but without mentioning a version using pip install, which are:

Sphinx
sphinx-rtd-theme
myst-parser

[prcutler]

Thanks - I created a new venv using the latest 3.9.12 and it worked! I added a missing word from the docs I wrote and pushed it. It should be ready for review and merging if the wording is acceptable.

Thanks again!

[AnssiAhola]

Copied from Review comment that for some reason is invisible to others 🤷

I think a better way to put this is to mention that there can be multiple instances of a given release in one or multiple collection folders and this new method is to make it more convenient to fetch all release instances from all collection folders.

Example:

It is possible that you have multiple instances of the same release inside one or across multiple collection folders and you might want to update or remove all of those instances.
You can fetch all instances of that release across all collection folders by release_id or Release object.

release_instances = me.collection_items(22155985)
for instance in release_instances:
    print(instance) # CollectionItemInstance

This method returns a list of CollectionItemInstance objects

Note.To remove an instance from the collection folder it belongs to, you need to first fetch the correct folder (using instance.folder_id) and remove it that way. See section "Removing a Release from a Collection Folder"

Something like that, but better phrased and such :)

Hope that clears some things.

@JOJ0 Did you have some other ideas about how to document this?

[JOJ0]

to me an important point is that with that feature we dont need to iterate through all folders to find a release, we can just get it (the colliteminstance incl all details like folder) by passing a release id we know we own.

i’m still not sure if i like the chapter title. i suggest: Getting a collection item via the release id, similar to the api endpoint docs.

[AnssiAhola]

Can we add a link to the correct section in the docs?
Although that section is right above this one so maybe not needed?

[JOJ0]

Agree that linking to the remove chapter is essential even though it's close.

We should omit the word "iterate" in the first paragraph, since this way the point is that we don't have to iterate as opposed to the other method (accessing by folder).

I propose we move the whole chapter right below the initial "collection data" chapter: https://python3-discogs-client.readthedocs.io/en/latest/fetching_data.html#collection-data

I rephrased and reordered the whole chapter to fit in the context of this new position:

Collection Items by Release

As we saw in the previous chapter (link to it even it's close) we can access collection items by iterating through a specific folder or through the whole collection.

An alternative approach is using the collection_items method which returns a list of CollectionItemInstance objects. We need to pass it a release_id or a Release object.

Since It is possible that you have (or even pysically own) multiple copies of the same release inside one or more
collection folders, using this method you'll receive all copies you own of that specific release in whatever folder it's located.

This example code prints us all CollectionItemInstances of release 22155985. Via the attributes of the CollectionItemInstances we can receive further information like the folder it's located in or get details of the actual release.

release_instances = me.collection_items(22155985)
for instance in release_instances:
    print(instance) 

Note: To remove an instance from the collection folder it belongs to, you need to fetch the
correct folder (using instance.folder_id) to remove it. See the section
"Removing a Release from a Collection Folder".

[JOJ0]

Also it would be nice if we directly link things like collection_items and CollectionItemInstance to the page/entry in the Python package docs. I forgot the syntax but have a look around, we did that already.

[JOJ0]

Nice! We are getting there. I hijacked the branch and pushed a commit. I moved the chapter up and also spotted some errors we had in the docs way before this PR (regarding folder id's 0, 1, 2). Have a look at the commit message. Pleas make sure you review the whole Collection Data chapter top to bottom in a locally rendered version.

[JOJ0]

Also I realized we have a mixture of writings/formatting of Release object release release, and stuff. We should get that straight. I promise I'll find out how to format to link to sphinx generated package docs methods, In a lot of places we could use that as well or instead of worrying about our "common" formatting for an Object. Here's an example but it's rather long/full path: https://python3-discogs-client.readthedocs.io/en/latest/authentication.html#more-information
Not sure if that can be shortened.

And something else: I spotted an error in our readthedocs build. It's the same Jinja error you had @prcutler . Should be easey to fix. Will look into it.

[JOJ0]

• I fixed the Jinja2 error by rebasing the deps_handling branch with fixed requirements files into it.
• I activated this PR's branch so we can have a look at the generated docs online: https://python3-discogs-client.readthedocs.io/en/pr91/fetching_data.html#collection-data
• I put an example of "a link to CollectionItemInstance Sphinx package docs" in the "Collection Data" intro chapter so you see how it would look like: It is rather longish and ugly to put in at every instance of CollectionItemInstance, so I suggest we should stick to our typical highlighting with backticks for object names and use this linking sparingly or just in places where it makes sense to have a long-detailed name and a link to sphinx reference docs.

Todo:

• Fix occurences of release with Release If you find the time @prcutler that would be great. Also look through and find other mixtures of formatting object names. I just did that, you can still look through I might have missed some. Thanks!
• I would love to find a way in Sphinx to abbreviate those reference links but I didn't manage to test whether this replacing feature would work with it or if the links would get broken when it's used???: https://myst-parser.readthedocs.io/en/v0.15.1/syntax/optional.html
• I did not test all the new and fixed codeblocks! Probably someone should do that....I'ltt try to find the time.....but if someone...? :-))
• Review and fix .fetch chapter, maybe @AnssiAhola wants to do that?

[JOJ0]

@prcutler I figured it out (second checkbox): Have a look at my latest commit. This is how we could directly link to eg. CollectionItemInstance, get nice formatting but with an abreviated form without the annoyingly long full path of discogs_client.models.CollectionItemInstance: Last sentence in this chapter: https://python3-discogs-client.readthedocs.io/en/pr91/fetching_data.html#collection-data

I think we should use this with the word "Release" (object) as well!

[JOJ0]

Added an example for a link to the Release object docs.

[JOJ0]

I replaced all instances of Client, Release, CollectionItemInstance and more classes and methods with links to the Sphinx reference docs of the Python package. I also found out that it's possible to link to single attributes of a class. See title at the end of this chapter: https://python3-discogs-client.readthedocs.io/en/pr91/fetching_data.html#collection-items-by-folder

[AnssiAhola]

Not sure how to link to a method in a specific class, since the fetch method is present in Fetchers as well as APIObjects and the docs seem to default to the Fetchers one

[JOJ0]

Yeah @AnssiAhola I stumbled across that as well, I just left it alone then because I was not sure which one is the correct one (and to lazy to investigate). So you are saying the one and only correct method we want to link to is the one in APIObjects?

[AnssiAhola]

Yeah @AnssiAhola I stumbled across that as well, I just left it alone then because I was not sure which one is the correct one (and to lazy to investigate). So you are saying the one and only correct method we want to link to is the one in APIObjects?

Yes the one in PrimaryAPIObject

[JOJ0]

@AnssiAhola have a look at my commit....You see you can put the whole path to a method but then to have it abbreviated as fetch you need to put the tilde ~ before it!
The think that I'm not sure about is that actually the method is a different one depending on what object it is used (Primary, Secondary). I linked to the one in PrimaryAPIObjects class on the first instance and to the one in SecondaryAPIObjects on the second instance of the word fetch(). Not sure if this is entirely correct now and also not sure if this could be confusing to the reader.

The "message" this chapter should transmit to the reader was simple before these links to fetch(): Any object -> u can use fetch(). Period. ;-)) Are we overcomplicating things now? Haha :-)

[AnssiAhola]

@JOJ0 Maybe the links are not necessary in this case since the example shows how to use it and the actual documentation for the method is not that useful?

[JOJ0]

Thanks for the last changes @AnssiAhola . This chapter reads very good and straight forward now IMO.

Thanks to both of you for a great team effort in this PR overall! I think we did a very good job explaining the new feature but improved this page a lot as a whole. I'll merge this now and we are ready for a release.