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

Update README #9

Merged
merged 17 commits into from
Jun 26, 2020
Merged

Update README #9

merged 17 commits into from
Jun 26, 2020

Conversation

ejulio
Copy link
Collaborator

@ejulio ejulio commented May 19, 2020

No description provided.

@ejulio ejulio requested review from kmike and Gallaecio May 19, 2020 11:15
@codecov
Copy link

codecov bot commented May 19, 2020

Codecov Report

Merging #9 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #9   +/-   ##
=======================================
  Coverage   98.00%   98.00%           
=======================================
  Files           4        4           
  Lines         251      251           
=======================================
  Hits          246      246           
  Misses          5        5           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc1af22...f34f25b. Read the comment docs.

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@ejulio ejulio requested a review from kmike May 21, 2020 10:32
README.rst Outdated Show resolved Hide resolved
Co-authored-by: Adrián Chaves <[email protected]>
README.rst Outdated
Comment on lines 26 to 33
``itemloaders`` is a library that helps you collect data into models.

It's specially useful when you need to standardize the data from many sources.
For example, it allows you to have all your casting and parsing rules in a
single place.

Also, it comes in handy to extract data from web pages, as it supports
data extraction using CSS and XPath Selectors.
Copy link
Member

@kmike kmike May 25, 2020

Choose a reason for hiding this comment

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

This description is a bit confusing to me. Are we suggesting that itemloaders is a general thing, not related to web scraping, which may also come handy for web scraping? Is it really going to be used this way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess it is not restricted to web scraping.
If I want to load a dict from a XML source, it could be used, right?
Similarly to read from a JSON source or something else..

So, we can have the description related to web scraping or leave it open as a library to standardize the process of extracting/loading data from a source

Copy link
Member

Choose a reason for hiding this comment

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

What about explicitly mentioning HTML and XML as the sources of data in the first paragraph, and in the third paragraph replace “comes in handy” with “is specially useful” and move the CSS and XPath part to the first paragraph?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated @Gallaecio

@ejulio
Copy link
Collaborator Author

ejulio commented Jun 1, 2020

@kmike @Gallaecio , any updates here?

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
Copy link
Member

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

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

The current text is fine by me.

I do see @kmike’s point about the introduction, though. Maybe the 2nd and 3rd paragraphs should be inverted, web scraping being what itemloaders is specially useful for, and data standarization being something for which itemloaders can also come in handy.

@ejulio
Copy link
Collaborator Author

ejulio commented Jun 23, 2020

hey @kmike
I'd updated this one with some of your thoughts a couple of days ago.
If you have some time, can you check it?

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
ejulio and others added 3 commits June 25, 2020 11:08
Co-authored-by: Mikhail Korobov <[email protected]>
Co-authored-by: Mikhail Korobov <[email protected]>
Co-authored-by: Mikhail Korobov <[email protected]>
@kmike kmike merged commit cc8ed6c into master Jun 26, 2020
@kmike
Copy link
Member

kmike commented Jun 26, 2020

Thanks @ejulio!

@ejulio ejulio deleted the feat-readme branch June 26, 2020 14:07
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.

3 participants