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.
|
|
||
| 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. 🤔
|
|
||
|
|
||
| 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.
| 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
2 participants
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)