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

document settings #722

Merged
merged 10 commits into from Oct 28, 2020
Merged

document settings #722

merged 10 commits into from Oct 28, 2020

Conversation

noviluni
Copy link
Collaborator

@noviluni noviluni commented Jun 25, 2020

I've been checking the current settings as a previous step to work on this: #630

I've been reordering the settings, reordering the settings in the docs, and checking the currently undocumented settings.

The undocumented settings are:
* SKIP_TOKENS_PARSER
* NORMALIZE
* FUZZY

I documented the NORMALIZE setting, however, after some checks, it seems that those settings are not documented because:
* It doesn't have any sense or use cases to change them (they were created because of legacy reasons).
* Nobody asked for it in issues, etc.

From this, I think that they are not useful at all, and it doesn't have any sense to keep them. For example, the default list in SKIP_TOKENS_PARSER can be set directly in the code, or NORMALIZE=True can be applied always as it is really specific (it only has sense when the checked source (CLDR json and yamls files) is not normalized, but if it's normalized it doesn't work).

My proposal is to remove them in the next major version but I need feedback on this. @Gallaecio

I documented all the undocumented settings and made some improvements to the documentation structure.

@noviluni noviluni requested a review from Gallaecio June 25, 2020 12:23
@noviluni noviluni changed the title document settings WIP: document settings Jun 25, 2020
@codecov
Copy link

codecov bot commented Jun 25, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #722   +/-   ##
=======================================
  Coverage   98.24%   98.24%           
=======================================
  Files         231      231           
  Lines        2569     2569           
=======================================
  Hits         2524     2524           
  Misses         45       45           
Impacted Files Coverage Δ
dateparser/__init__.py 100.00% <ø> (ø)
dateparser/conf.py 100.00% <ø> (ø)
dateparser/date.py 99.57% <ø> (ø)

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 2908d21...255ae51. Read the comment docs.

@noviluni
Copy link
Collaborator Author

noviluni commented Jun 25, 2020

Proposal for deleting SKIP_TOKENS_PARSER: #728

@Gallaecio
Copy link
Member

I think removing them makes sense.

@noviluni
Copy link
Collaborator Author

noviluni commented Sep 8, 2020

I will wait until merging this: #779
to avoid conflicts

This was referenced Sep 24, 2020
@noviluni noviluni marked this pull request as draft September 28, 2020 11:46
@noviluni noviluni modified the milestones: v1.0.0, 1.1.0 Oct 6, 2020
@noviluni noviluni mentioned this pull request Oct 26, 2020
@@ -1,5 +0,0 @@
Configurations
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed as it was not used

@@ -0,0 +1,189 @@
.. _settings:
Copy link
Collaborator Author

@noviluni noviluni Oct 27, 2020

Choose a reason for hiding this comment

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

Added a specific section for the settings (extracted from usage.rst). Multiple error fixes and improvements have been performed.

Comment on lines -11 to -25
The instance of :class:`DateDataParser <dateparser.date.DateDataParser>` reduces the number
of applicable languages, until only one or no language is left. It
assumes the previously detected language for all the subsequent dates supplied.

This class wraps around the core :mod:`dateparser` functionality, and by default
assumes that all of the dates fed to it are in the same language.

.. autoclass:: dateparser.date.DateDataParser
:members: get_date_data

.. warning:: It fails to parse *English* dates in the example below, because *Spanish* was detected and stored with the ``ddp`` instance:

>>> ddp.get_date_data('11 August 2012')
{'date_obj': None, 'period': 'day'}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not true now, so I removed it.

* `SKIP_TOKENS`
* `NORMALIZE`
* `RETURN_TIME_AS_PERIOD`
* `PARSERS`
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 added all the missing settings and removed the descriptions because it makes this harder to maintain. For up to date descriptions we have the settings section in docs.

@@ -268,7 +268,7 @@ class DateDataParser:

:param locales:
A list of locale codes, e.g. ['fr-PF', 'qu-EC', 'af-NA'].
The parser uses locales to translate date string.
The parser uses only these locales to translate date string.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clarified because of the comment here: #789 (comment)

@noviluni noviluni marked this pull request as ready for review October 27, 2020 13:38
@noviluni noviluni changed the title WIP: document settings document settings Oct 27, 2020
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.

Nice!

@noviluni noviluni merged commit bc0424a into scrapinghub:master Oct 28, 2020
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.

None yet

2 participants