[PATCH] D68800: [LNT] Python 3 support: adapt csv reader open

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre marked an inline comment as done.
thopre added inline comments.


================
Comment at: lnt/tests/nt.py:774-777
+    if sys.version_info[0] < 3:
+        report_file = open(report_path, 'rb')
+    else:
+        report_file = open(report_path, 'r', newline='')
----------------
PrzemekWirkus wrote:
> PrzemekWirkus wrote:
> > PrzemekWirkus wrote:
> > > PrzemekWirkus wrote:
> > > > PrzemekWirkus wrote:
> > > > > Can you verify that you should rather use mode == 'U' for universal mode?
> > > > > 
> > > > > https://docs.python.org/release/3.2/library/functions.html#open
> > > > > 
> > > > > See table below the Python 3 open function instead of newline == ''.
> > > > > 
> > > > > I'm not an expert in this area.
> > > > I need to clarify:
> > > > 
> > > > I was wondering if we should combine mode == 'rU' with newline=='' or not.
> > > My thinking is that when you use Universal mode "rU" you will get all newlines -> '\n'. Would that be enough you think?
> > ```
> > report_file = open(report_path, 'ru')
> > ```
> > 
> > Should be enough I guess. Are you able to verify this on your end ?
> s/u/U/
> 
> ```
> report_file = open(report_path, 'rU')
> ```
It works on my testing but I'm not too keen on using rU since the doc you linked says specifically it's for compatibility and should not be used in new code. My goal in this Python 3 conversion is to follow the Python 3 codestyle/idioms.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68800/new/

https://reviews.llvm.org/D68800