pystac

pystac copied to clipboard

@gadomski prefers importing classes directly, but not functions. E.g.

import pystac.utils
from pystac import Item

item = Item.from_path("item.json"0
datetime_str = pystac.utils.str_to_datetime("2023-10-12T09:00:00Z")

Happy to arguments for other syles, but once we settle on one, we should normalize the repo. Maybe there's an automated tool to do this? If not, it'll be some manual choreing.

Ok I have thoughts.

For internal usage I think we should import the minimum and from the file where the definition is. So I'd prefer:

from pystac.utils import str_to_datetime
from pystac.item import Item

item = Item.from_path("item.json")
datetime_str = str_to_datetime("2023-10-12T09:00:00Z")

For external usage (I include tests in this) I went to see what pep8 says. It is pretty generous:

When importing a class from a class-containing module, it’s usually okay to spell this:

from myclass import MyClass
from foo.bar.yourclass import YourClass

If this spelling causes local name clashes, then spell them explicitly:

import myclass
import foo.bar.yourclass

So for pystac, I agree that the big top-level classes (Asset, Item, Collection, ItemCollection, Catalog) should all be ok to directly import but then I look at pystac-client and I want to follow the same best practices but I really don't want confusion between pystac_client.Client and dask.distributed.Client. Plus I probably want the top level functions off of pystac anyways. Things like pystac.read_file so I end up importing pystac anyways.

from pystac.utils import str_to_datetime
from pystac.item import Item

item = Item.from_path("item.json")
datetime_str = str_to_datetime("2023-10-12T09:00:00Z")

I'd be ok with this -- issues can arise if two modules have functions with the same name, but I think PySTAC's currently built so that's not really a problem. I'll 👍🏼 this idea.

I really don't want confusion between pystac_client.Client and dask.distributed.Client

Agreed, and I don't think we need to recommend how people use PySTAC -- I was mostly focusing on the interal usage.