EXACTsports / roster-project

Experimenting with scraping college team rosters
0 stars 0 forks source link

Review Albert's Approach #2

Closed barrytarter closed 1 year ago

barrytarter commented 1 year ago

@edgrosvenor can you quickly review Albert's work (@hardcommitoneself ) and give him some guidance. He shared this video https://www.loom.com/share/262f7d29525f45eba0caa4e8455a965d which shows a UI he's started. I'm not quite sure what we are looking at there, but you might know better.

@hardcommitoneself can you share an example CSV of a full team roster -- one that has a lot of data / variables / columns? Were you able to map things? Please attach other information into here on your approach so @edgrosvenor can review your work so far.

hardcommitoneself commented 1 year ago

@barrytarter

For now, I just focused on stable and scalable of our basic scrapper. So this scraper can scrap data from almost any URL mentioned in our old DB. And I don't think it's a problem to collect different data for each roster. I will work on it from now. So that we can get a lot of data of each athlete. athletes.csv

edgrosvenor commented 1 year ago

The video shows a sensible UI and the spreadsheet has a decent looking set of data. I'd be curious to look at the code to see how it was done.

hardcommitoneself commented 1 year ago

@edgrosvenor

I already pushed all changes. I think you can take a look at the code base. Please let me know if you need any help from me. And if you want, I will describe approach for you.

edgrosvenor commented 1 year ago

@hardcommitoneself Looks good so far. The only big thing I've seen is that you combine properties on a single line. I like to give each property its own line. I'll close this. Just tag me in a comment on the PR I opened for your branch when you have more stuff you want me to review.

hardcommitoneself commented 1 year ago

@edgrosvenor

Could you let me know more detail about The only big thing I've seen is that you combine properties on a single line.?

edgrosvenor commented 1 year ago

@hardcommitoneself https://github.com/EXACTsports/roster-project/blob/b794dabe821b24022b2079ff6e17e6d4f9e8e3af/app/Http/Livewire/Rosters.php#L18

public $rosters = [], $file, $loadData = false, $selectedAthletes = [], $selectedRoster = null, $first_id = -1, $end_id = -1;

should be written

public $rosters = [];
public $file;
public $loadData  = false;
// ...etc.
hardcommitoneself commented 1 year ago

Ah, makes sense. you meant the variables. Will fix it.