def read_portfolio(filename):
        with open(filename) as f:
            rows = csv.reader(f)
            headers = next(rows)
            for row in rows:
                yield {
                    'name' : row[0],
                    'shares' : int(row[1]),
                    'price' : float(row[2]),
                }

    portfolio = read_portfolio()
    for record in portfolio:
        print '{shares} shares of {name} at ${price}'.format_map(record)

Personally the way you've done it is the most Pythonic IMO. List comprehensions are great but would be less readable in this case.

You could use a list comprehension, but that can be unclear and hard to extend, depending on the situation. It can be a nice option if most of the parts in the generator can be broken out into functions with their own name, though.

You could turn it into a generator, which can cause some fun bugs (e.g. everything works fine when you first iterate over it, but not afterwards), so IMO that's best used when it needs to be a generator, for semantics or performance.

You could turn it into a generator, then add a wrapper that turns it into a list (keeping the inner function private), or use a decorator that does the same, but it's less clear than this pattern.

So, i'd just learn to live with it.

Yeah, so many people don't get this, but too many small functions can be hard to understand -- that's why I qualified that option.

In this case i agree that inlining it is fine, i was talking about the general pattern.

So `portfolio = [{'name': row[0], 'shares': int(row[1]), 'price': float(row[2]) for row in rows]`

But if it's more complicated than this (like if there is conditional(s) inside the loop), I'd recommend just stick with the current approach. It's possible to have even multiple conditionals in list comprehension, but it's not really very readable. If you do want to, walrus operator can make things better

(something like `numbers = [m[1] for s in array if (m := re.search(r'^.*(\d+).*$', s))]`)

    keys = "name", "shares", "price"
    portfolio = [
        dict(zip(keys, row))
        for row in rows
    ]

That said, it's fine to append to a list too, I just prefer comprehensions when they fit the job. In particular, if you're just going to iterate once over this list anyway, you can turn it into a iterator comprehension by replacing [] by () and save some memory.

    def get_record(row):
        return {
                'name': row[0],
                'shares': int(row[1]),
                'price': float(row[2])
        }
    return [ get_record(r) for r in rows ]

    return list(map(get_record, rows))

In 2023, it's rude to return dictionaries. :)

Why do you say that?

I think you were partially kidding, but also half serious. What’s the issue with returning dictionaries, and why should we be returning dataclasses instead?

Asking for my own learning.

  def some_method(input: Dict[str, int]):
    ...

vs.

  def some_method(input: MyDataClassWithFoo):
    ...

Regarding rude, yea it was a bit tongue-in-cheek (I would never hold a grudge against somebody for returning a dict).

You should generally define an interface for your function that's as precise as its logic allows for. `Dict` is as good as `Any`: it doesn't tell you very much about what the function internally expects. The sibling comment to mine does a good job going through this.

    def iter_portfolio(rows):
         for row in rows:
             yield {'name': row[0]}
    
    rows = ...
    portfolio = list(iter_portfolio(rows))

Honestly, this is the approach I've been using even though I hate it. Specially if your code is going to be read by anyone other than you.

    with open(filename) as f:
        rows = csv.reader(f)
        next(rows)
        return [
            {
                'name': row[0],
                'shares': int(row[1]),
                'price': float(row[2]),
            } for row in rows
        ]

    def read_portfolio(filename):
        record = lambda r: {
            'name': r[0],
            'shares': int(r[1]),
            'price': float(r[2]),
        }
        with open(filename) as f:
            rows = csv.reader(f)
            headers = next(rows)
            return [record(r) for r in rows]

That will read the file as needed (ie as you iterate over it) instead of loading the entire thing in memory.

    for record in read_portfolio(fn):
        # do stuff

  def read_portfolio(filename):
      with open(filename) as f:
          rows = csv.reader(f)
          headers = next(rows)
          return [
              {
                  "name": r[0],
                  "shares": int(r[1]),
                  "price": float(r[2]),
              }
              for r in rows
          ]

    header, *records = [row.strip().split(',') for row in open(filename).readlines()]

    type_record = lambda r : (r[0], int(r[1]), float(r[2]))

In the course example, it's reasonably easy to add some try blocks/error handling/default values while assigning records, giving you the chance to salvage valid rows without affecting speed or readability. In fact, error handling would be a necessity if that CSV file is externally accessible. Contrast that with my two lines, where there's not an elegant way to handle a bad row or escaped comma or missing file or virtually any other surprise.

Anything else I can think of off-hand (defaultdict, UserList, "if not portfolio:") has the same initialization step, endures some performance degradation, is more fragile, and/or is needlessly unreadable, like this lump of coal:

    portfolio = [record] if 'portfolio' not in globals() else portfolio + [record]

> header, *records = [row.strip().split(',') for row in open(filename).readlines()]

Better would be:

    header, *records = [row.strip().split(',') for row in open(filename)]

Edit: Also if you want to be explicit with the file closing, you could do something like:

    with open(filename) as infile:
        header, *records = [row.strip().split(',') for row in infile]

It looks like that code does read the whole file:

(with a foo.csv that is 350955 bytes long:)

  % python -V
  Python 3.11.4
  % python
  >>> f = open("foo.csv")
  >>> f.tell()
  0
  >>> header, *records = [row.strip().split(',') for row in f]
  >>> f.tell()
  350955

  >>> f.close()
  >>> f.open("foo.csv")
  >>> header, *records = (row.strip().split(',') for row in f)
  >>> f.tell()
  350955

  >>> f.close()
  >>> f.open("foo.csv")
  >>> headers, records = f.readline().strip().split(','), (row.strip().split(',') for row in f)
  >>> f.tell()
  125

    [... for row in open(filename).readlines()]

    [... for row in open(filename)]

Additionally, this doesn't do what you think it does:

    >>> header, *records = (row.strip().split(',') for row in f)

    >>> gen = (row.strip().split(',') for row in f)
    >>> header, *records = next(gen)

``` There should be one-- and preferably only one --obvious way to do it. ```