C22 - Sphinx - Priyanka and Tatiana

[Nerpassevera]

No description provided.

[Nerpassevera]

Alternative function implementations created after discovering higher-order functions:

    def get_by_id(self, item_id):
        result = next(filter(lambda item: item.id == item_id, self.inventory), None)
        return result

    def get_by_category(self, category):
        result = list(filter(lambda item: item.get_category() == category.capitalize(), self.inventory))
        return result

    def get_best_by_category(self, category):
        items_in_category = self.get_by_category(category)
        if items_in_category:
            result = max(items_in_category, key=lambda item: item.condition, default=None)
            return result
            ```

[Nerpassevera]

We practiced writing docstrings for classes and methods. @yangashley, could you please give us feedback on this subject too?

[yangashley]

@Nerpassevera When writing docstrings you should adhere to some style for consistency. What style is used depends on team preferences so I can't speak to which specific style you should use.

Docstrings should ideally contain all information that is relevant for using that class or method. Normally, that is less about describing the details of how the code works, and more about describing the purpose and general behavior.

A couple example of guides:

• https://pandas.pydata.org/docs/development/contributing_docstring.html
• https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings

[yangashley]

Nice job on swap-meet, Priyanka and Tatiana!

I can see from the commit history that you made frequent commits, which is great!

Let me know if you have any questions about my comments.

[yangashley]

Was this change accidentally committed?

If this change wasn't needed then take care to remove it from the list of files being added and committed. You can unstage a file for commit if you haven't run git commit yet and you can look at how to locally revert a commit if you already added/committed the file.

In industry, when you write code and open a PR it means you're asking to merge code into the main branch. Therefore, you will not be permitted to merge in changes that are not part of your feature work because it will cause noise in the commit history or bring in unnecessary changes to the codebase.

It's best practice to run git status to see what changes will be committed before you commit so you can catch stray changes like this.

[yangashley]

If you were using Google's docstring guide for Python, then you can follow the info found in the guide here about writing docstrings for a class

[yangashley]

Nice work importing Item since we're using by name as the parent class of Clothing 👍

[yangashley]

Nice work on here!

On line 23 you correctly set the default value for fabric to "Unknown".

On line 24 you properly call the initializer of the parent, and pass along the values the child wants to use.

Lastly, good work remembering to refactor the function definition to include age so that it can get set from the subclass

[yangashley]

You could also do a one line return like:

f"{super().__str__()} It is made from {self.fabric} fabric."

[yangashley]

This assertions will fail if the order of items in each list somehow change. The README didn't specify that the order of inventory items needed to be maintained, so we probably don't need to constrain our tests and therefore our functions to these requirements. In fact, if you were to implement the swap that I suggested in a comment using multiple assignment, then these tests could fail because the order of elements would be changed.

While more verbose, you can check that elements are in a list like:

    assert item_d in jesse.inventory
    assert item_e in jesse.inventory
    assert item_c in jesse.inventory

And since we're checking Jesse's inventory, it would be make the test more thorough to also check the items in Tai's inventory too.

[yangashley]

swap_best_by_category should return False if a swap doesn't happen. More than be falsy, result is intended to return an actual boolean. So while we typically write something like

    assert not result

in regular python, here we might prefer the more explicit check of

    assert result == False

Same comment applies to similar assertion written in another test below.

[yangashley]

Do we need to consider order when checking the inventories of these vendors? If not, we don't need to test for it here.

Same comment applies to similar assertions written in another test below.

[yangashley]

Remove debugging print statements when you're finished with them so they're not part of the PR.

[yangashley]

Nit: inconsistent whitespaces around square brackets.

Suggested change

	assert tai.inventory == [ item_b, item_a, item_f]
	assert jesse.inventory == [ item_e, item_d, item_c ]
	assert tai.inventory == [item_b, item_a, item_f]
	assert jesse.inventory == [item_e, item_d, item_c]

[PriyankaKaramchandani]

@yangashley @Nerpassevera: Ashley I wanted to ask: when ever I want to pull changes of my team it asks me to always make my commits first. Only if I have made commits it allows me to proceed with git pull.
I read about stashing my changes with: git stash or git stash pull. Should I be using that?