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.

Sign up for GitHub

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

Jump to bottom

Fix listing of example directory #6

Closed
wants to merge 3 commits into from
Closed

Fix listing of example directory #6

wants to merge 3 commits into from

Conversation

thekaveman
Copy link
Member

@thekaveman thekaveman commented Jun 23, 2021 •
edited
Loading

New Pull Request

What existing problem does the pull request solve and why should we include it?

By the time this line executes, we've already determined example_dir does not exist, so there is no sense in trying to list it.

Instead, list the parent directory main_example_dir, which (presumably) contains some specific example directories.

What is the testing plan?

Demonstrate the code is solid by discussing how results are verified and covered by tests.

  • Code for this PR is covered in tests
  • Code passes all existing tests
  • Code is attempting to make existing tests pass

Code formatting

Code should be PEP8 compliant before merging by running a package like black

  • Code linted

Applicable Issues

Please do not create a Pull Request without creating an issue first.

Issues List

Closes #5

@thekaveman
fix(io): list main_example_dir
74dd2b1 
by the time this line executes, we've already determined
example_dir doesn't exist, no sense in trying to list it

instead, list the parent example dir
@thekaveman thekaveman marked this pull request as draft June 25, 2021 16:35
@e-lo
[fix] Add missing examples files
1c4095b 
- Add missing example/prototype .csv files
- Update .gitignore so examples aren't ignored

Fixes #6
@e-lo
Copy link
Collaborator

e-lo commented Jun 29, 2021

@thekaveman - wondering why the pre-commit checks didn't bomb on my local commit, but are in GH Actions? Is there something in the setup that is causing it to skip locally?

@thekaveman
Copy link
Member Author

@e-lo I'm not sure? I pulled your commit in and ran pre-commit run --all --all-files and it bombed the same way.

I also tried renaming the .csv files to get rid of spaces etc. and when I committed that change, pre-commit also complained. Besides the line-endings/extra spaces/etc. there's some funky metadata or something at the top of alot of these files - is that correct?

E.g. first row from components.csv looks like a description or something?

Key: pcc_id, a concatenation of Product, Component, and Contract ID.
A many-to-one relationship with contracts using the foreign key: Contract ID",,,,,,,,,
pcc_id,Product,Component,Contract ID,Transit Provider,Vendor,~Capital Service Entry Date,~Capital EOL Date,Notes,Version
Token Transit Mobile Ticketing|Mobile ticketing|Token Transit|GET Bus 1,Token Transit Mobile Ticketing,Mobile ticketing,GET Bus 1,GET Bus,Token Transit,,,,
TO CONFIRM|Farebox|Genfare|GET Bus 2,TO CONFIRM,Farebox,GET Bus 2,GET Bus,Genfare,,,,

Can I clean that content up as well?

@e-lo
[fix] Update local example to be pure csv
dcb6114 
csv files in the examples/prototype directory had a metadata header which was potentially the problem with passing pre-commit checks.

Removed header so they are now pure csv and updated `read_stack_from_dir()` appropriately.
@e-lo
Copy link
Collaborator

e-lo commented Jun 30, 2021

first row from components.csv looks like a description or something?

Yeah - it is a direct download from the transit stacks google sheet. But...for the purposes of this repo I think we should just assume pure csv (more consistent with getting it on GCS anyway).

@thekaveman thekaveman marked this pull request as ready for review June 30, 2021 18:52
@thekaveman
Copy link
Member Author

thekaveman commented Jun 30, 2021 •
edited
Loading

It looks like tests are now passing @e-lo! (And pre-commit checks)

I'll take a look at #7 / the missing /icons next. I think this is good to merge since we fixed the initial bug with transitstacks.io.example_dir() and got the examples directory back in there.

@thekaveman thekaveman requested a review from e-lo June 30, 2021 19:05
@thekaveman thekaveman closed this Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@e-lo e-lo Awaiting requested review from e-lo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] transitstacks.io.example_dir() loops over non-existent dir
2 participants
@thekaveman @e-lo