remove some python2 specific code #749
Conversation
Codecov Report
@@ Coverage Diff @@
## master #749 +/- ##
==========================================
+ Coverage 95.21% 95.57% +0.36%
==========================================
Files 304 304
Lines 2632 2600 -32
==========================================
- Hits 2506 2485 -21
+ Misses 126 115 -11
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Regarding the backward-incompatible changes in dateparser.utils, it could be argued that these are internal functions. However, given they are not prefixed with _ and they are listed in the documentation, it may be better to deprecate instead of remove.
dateparser/search/search.py
Outdated
| @@ -180,7 +174,7 @@ def __init__(self): | |||
| self.search = ExactLanguageSearch(self.loader) | |||
|
|
|||
| def detect_language(self, text, languages): | |||
| if isinstance(languages, (list, tuple, Set)): | |||
| if isinstance(languages, (list, tuple, set)): | |||
There was a problem hiding this comment.
I think this is technically a backward-incompatible change, as it is technically possible for an object to be an instance of a class inheriting from Set that is not a set.
There was a problem hiding this comment.
I change it because we accepted as valid this other PR: #509
but now I'm thinking that we should probably keep using Set as it would accept a frozenset for example.
>>> a = frozenset([1,2,3])
>>> isinstance(a, Set)
True
>>> isinstance(a, set)
FalseSo it is not only "backward-incompatible", but also not desirable. 🤔
| @@ -18,9 +18,6 @@ def strip_braces(date_string): | |||
|
|
|||
|
|
|||
| def normalize_unicode(string, form='NFKD'): | |||
| if isinstance(string, bytes): | |||
There was a problem hiding this comment.
As we are going to release a new major version I think it's a good moment to make these backward-incompatible changes. What do you think?
Apart from that, we could also move some of these functions to the private scope by adding a _.
There was a problem hiding this comment.
Either works. We just need to take this into account when writing the release notes, to make sure we cover backward-incompatible changes.
| @@ -44,25 +41,6 @@ def combine_dicts(primary_dict, supplementary_dict): | |||
| return combined_dict | |||
|
|
|||
|
|
|||
| def convert_to_unicode(info): | |||
|
I reverted the change in the @Gallaecio, could you check it again? |
noviluni
force-pushed
the
remove_python2_code
branch
from
ac60a7d to
4ef94a3
Compare
noviluni
force-pushed
the
remove_python2_code
branch
from
4ef94a3 to
7bec52e
Compare
I fixed some cases with Python 2 specific code that were detected when removing Python 2 from the pipelines (#744) (https://codecov.io/gh/scrapinghub/dateparser/pull/744/changes)