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

feat: add GTFS downloader #202

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

piotrgramacki
Copy link
Collaborator

closes #175

@piotrgramacki piotrgramacki self-assigned this Mar 12, 2023
@piotrgramacki piotrgramacki linked an issue Mar 12, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Mar 12, 2023

Codecov Report

Attention: 55 lines in your changes are missing coverage. Please review.

Comparison is base (b9894e9) 88.15% compared to head (f26a41b) 90.99%.

❗ Current head f26a41b differs from pull request most recent head a634b2c. Consider uploading reports for the commit a634b2c to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
+ Coverage   88.15%   90.99%   +2.84%     
==========================================
  Files          66       49      -17     
  Lines        2929     1444    -1485     
==========================================
- Hits         2582     1314    -1268     
+ Misses        347      130     -217     
Flag Coverage Δ
macos-latest-python3.10 90.99% <1.78%> (?)
macos-latest-python3.11 ?
ubuntu-latest-python3.10 90.92% <1.78%> (+3.45%) ⬆️
ubuntu-latest-python3.11 ?
ubuntu-latest-python3.8 90.90% <1.78%> (+3.38%) ⬆️
ubuntu-latest-python3.9 90.97% <1.78%> (+3.51%) ⬆️
windows-latest-python3.11 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
srai/loaders/osm_loaders/pbf_file_downloader.py 55.14% <ø> (-33.86%) ⬇️
srai/downloaders/__init__.py 0.00% <0.00%> (ø)
srai/utils/download.py 36.36% <33.33%> (ø)
srai/downloaders/gtfs_downloader.py 0.00% <0.00%> (ø)

... and 73 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Calychas Calychas added Skip-Changelog Skip changelog enforcer and removed Skip-Changelog Skip changelog enforcer labels Jul 5, 2023
Copy link
Collaborator

@RaczeQ RaczeQ Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't make another namespace / module, since PbfFileDownloader and other classes related to pbf files are next to OsmPbfLoader. I'd even go further and merge the logic of GTFSDownloader with GTFSLoader to simplify the usage for the user. Maybe create a directory gtfs within loaders and place both files there.

By this I mean modyfing existing function:

@overload
def load(
        self,
        area: Union[BaseGeometry, Iterable[BaseGeometry], gpd.GeoSeries, gpd.GeoDataFrame],
        fail_on_validation_errors: bool = True,
        skip_validation: bool = False,
    ) -> gpd.GeoDataFrame:
    ...

@overload
def load(
        self,
        gtfs_file: Union[str, "os.PathLike[str]"] = None, # notice the change of input type, we should accept both types
        fail_on_validation_errors: bool = True,
        skip_validation: bool = False,
    ) -> gpd.GeoDataFrame:
    ...

def load(
        self,
        area: Optional[Union[BaseGeometry, Iterable[BaseGeometry], gpd.GeoSeries, gpd.GeoDataFrame]] = None,
        gtfs_file: Optional[Union[str, "os.PathLike[str]"]] = None, # notice the change of input type, we should accept both types
        fail_on_validation_errors: bool = True,
        skip_validation: bool = False,
    ) -> gpd.GeoDataFrame:
    # check if area and gtfs_file are both None or both not None to raise an error
    # if area is not none then download a gtfs file and proceed (or throw an error if not found)
    # if file path - just proceed

@@ -7,33 +7,34 @@


def download_file(
url: str, fname: str, chunk_size: int = 1024, force_download: bool = True
url: str, filename: Path, chunk_size: int = 1024, force_download: bool = True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't force users to use only string or only Path, we can accept an input like this: Union[str, "os.PathLike[str]"] and then wrap it with Path(...) at the start of the function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option for automated gtfs downloading
3 participants