Repository: aiidateam/qe-tools (language: python)

Issue:
Parsing PW input file
Hello

I tried to parse an input file:

```
from qe_tools import PwInputFile

with open('aiida.in', 'r') as in_f:
    parsed_file = PwInputFile(in_f.read())
parsed_file.get_structure_from_qeinput()
```
and got the error that `CELL_PARAMETERS` is a nonetype.

The solution was proposed that my `CELL_PARAMETERS` values did not contain 0.0 instead of a 0.

Worked:
```
CELL_PARAMETERS angstrom
16.1762 0.0 0.0000000000
-8.0881 14.009 0.0000000000
0.0 0.0 18.3843
```

Failed:
```
CELL_PARAMETERS angstrom
16.1762 0 0.0000000000
-8.0881 14.009 0.0000000000
0 0 18.3843
```


Discussion / hints:
@giovannipizzi @sphuber do you know if that is a limitation that QE itself puts on the input file, or is it just a limitation of our parser?
To be honest I don't know. I have not written these parser nor ever worked with them. But looking at `_parse_cell_parameters` which defines the regex that parses the `CELL_PARAMETERS` block, the regex seems to allow `0`, i.e. it considers the dot and decimals optional. So I am not sure why it fails just from looking at the code quickly. However, I would say that specifying an integer should work just fine and not return `None`.
> To be honest I don't know. I have not written these parser nor ever worked with them.

Yeah, same here 😅

Also looking at the code quickly, it seems the `cell_vector_regex` allows integers:

https://github.com/aiidateam/qe-tools/blob/359579ee4a88fb6920270de39f49524e9365fda2/qe_tools/parsers/_input_base.py#L595-L613


But the `cell_parameters_block_re` does not:

https://github.com/aiidateam/qe-tools/blob/359579ee4a88fb6920270de39f49524e9365fda2/qe_tools/parsers/_input_base.py#L546-L592
 
If that one doesn't match, it never gets to trying to parse the individual vectors. Not sure why there are these two separate regex-es, instead of just using the matches from  `cell_parameters_block_re`.
This was originally written by @lekah 
I think that we should accept ints (and convert them to floats)


Fix patch:
diff --git a/src/qe_tools/parsers/_input_base.py b/src/qe_tools/parsers/_input_base.py
index 30dfd64..e2f264a 100644
--- a/src/qe_tools/parsers/_input_base.py
+++ b/src/qe_tools/parsers/_input_base.py
@@ -14,10 +14,24 @@
 from .._qe_version import parse_version
 from ..exceptions import InputValidationError, ParsingError
 
-RE_FLAGS = re.M | re.X | re.I
+RE_FLAGS = re.MULTILINE | re.VERBOSE | re.IGNORECASE
 
 __all__ = tuple()  # type: Tuple[str, ...]
 
+NUMBER_PATTERN = r"""
+(?:
+   [-|+]?   # Plus or minus in front of the number (optional)
+   (?:\d*   # optional decimal in the beginning .0001 is ok, for example
+   [\.]     # There has to be a dot followed by
+   \d+)     # at least one decimal
+   |        # OR
+   (?:\d+   # at least one decimal, followed by
+   [\.]?    # an optional dot
+   \d*)     # followed by optional decimals
+   (?:[E|e|d|D][+|-]?\d+)?  # optional exponents E+03, e-05, d0, D0
+)
+"""
+
 
 class _BaseInputFile:
     """
@@ -373,34 +387,20 @@ def str01_to_bool(s):
     # Define re for the card block.
     # NOTE: This will match card block lines w/ or w/out force modifications.
     atomic_positions_block_re = re.compile(
-        r"""
+        fr"""
         ^ \s* ATOMIC_POSITIONS \s*                      # Atomic positions start with that string
-        [{(]? \s* (?P<units>\S+?)? \s* [)}]? \s* $\n    # The units are after the string in optional brackets
-        (?P<block>                                  # This is the block of positions
+        [{{(]? \s* (?P<units>\S+?)? \s* [)}}]? \s* $\n    # The units are after the string in optional brackets
+        (?P<block>                                      # This is the block of positions
             (
                 (
                     \s*                                 # White space in front of the element spec is ok
                     (
-                        [A-Za-z]+[A-Za-z0-9]{0,2}       # Element spec
+                        [A-Za-z]+[A-Za-z0-9]{{0,2}}     # Element spec
                         (
                             \s+                         # White space in front of the number
-                            [-|+]?                      # Plus or minus in front of the number (optional)
-                            (
-                                (
-                                    \d*                 # optional decimal in the beginning .0001 is ok, for example
-                                    [\.]                # There has to be a dot followed by
-                                    \d+                 # at least one decimal
-                                )
-                                |                       # OR
-                                (
-                                    \d+                 # at least one decimal, followed by
-                                    [\.]?               # an optional dot ( both 1 and 1. are fine)
-                                    \d*                 # And optional number of decimals (1.00001)
-                                )                        # followed by optional decimals
-                            )
-                            ([E|e|d|D][+|-]?\d+)?       # optional exponents E+03, e-05
-                        ){3}                            # I expect three float values
-                        ((\s+[0-1]){3}\s*)?             # Followed by optional ifpos
+                            {NUMBER_PATTERN}
+                        ){{3}}                          # I expect three float values
+                        ((\s+[0-1]){{3}}\s*)?           # Followed by optional ifpos
                         \s*                             # Followed by optional white space
                         |
                         \#.*                            # If a line is commented out, that is also ok
@@ -434,30 +434,27 @@ def str01_to_bool(s):
 
     # Define re for atomic positions without force modifications.
     atomic_positions_w_constraints_re = re.compile(
-        r"""
-        ^                                       # Linestart
-        [ \t]*                                  # Optional white space
-        (?P<name>[A-Za-z]+[A-Za-z0-9]{0,2})\s+   # get the symbol, max 3 chars, starting with a char
-        (?P<x>                                  # Get x
-            [\-|\+]?(\d*[\.]\d+ | \d+[\.]?\d*)
-            ([E|e|d|D][+|-]?\d+)?
+        fr"""
+        ^                                        # Linestart
+        [ \t]*                                   # Optional white space
+        (?P<name>[A-Za-z]+[A-Za-z0-9]{{0,2}})\s+ # get the symbol, max 3 chars, starting with a char
+        (?P<x>                                   # Get x
+            {NUMBER_PATTERN}
         )
         [ \t]+
-        (?P<y>                                  # Get y
-            [\-|\+]?(\d*[\.]\d+ | \d+[\.]?\d*)
-            ([E|e|d|D][+|-]?\d+)?
+        (?P<y>                                   # Get y
+            {NUMBER_PATTERN}
         )
         [ \t]+
-        (?P<z>                                  # Get z
-            [\-|\+]?(\d*[\.]\d+ | \d+[\.]?\d*)
-            ([E|e|d|D][+|-]?\d+)?
+        (?P<z>                                   # Get z
+            {NUMBER_PATTERN}
         )
         [ \t]*
-        (?P<fx>[01]?)                           # Get fx
+        (?P<fx>[01]?)                            # Get fx
         [ \t]*
-        (?P<fy>[01]?)                           # Get fx
+        (?P<fy>[01]?)                            # Get fx
         [ \t]*
-        (?P<fz>[01]?)                           # Get fx
+        (?P<fz>[01]?)                            # Get fx
         """, re.X | re.M
     )
     # Find the card block and extract units and the lines of the block.
@@ -542,39 +539,21 @@ def _parse_cell_parameters(txt):
     """
     # Define re for the card block.
     cell_parameters_block_re = re.compile(
-        r"""
+        fr"""
         ^ [ \t]*
         CELL_PARAMETERS [ \t]*
-        [{(]? \s* (?P<units>[a-z]*) \s* [)}]? \s* [\n]
+        [{{(]? \s* (?P<units>[a-z]*) \s* [)}}]? \s* [\n]
         (?P<block>
         (
             (
-                \s*             # White space in front of the element spec is ok
+                \s*             # White space in front is ok
                 (
-                    # First number
                     (
-                        [-|+]?   # Plus or minus in front of the number (optional)
-                        (\d*     # optional decimal in the beginning .0001 is ok, for example
-                        [\.]     # There has to be a dot followed by
-                        \d+)     # at least one decimal
-                        |        # OR
-                        (\d+     # at least one decimal, followed by
-                        [\.]?    # an optional dot
-                        \d*)     # followed by optional decimals
-                        ([E|e|d|D][+|-]?\d+)?  # optional exponents E+03, e-05, d0, D0
-
-                        (
-                            \s+      # White space between numbers
-                            [-|+]?   # Plus or minus in front of the number (optional)
-                            (\d*     # optional decimal in the beginning .0001 is ok, for example
-                            [\.]     # There has to be a dot followed by
-                            \d+)     # at least one decimal
-                            |        # OR
-                            (\d+     # at least one decimal, followed by
-                            [\.]?    # an optional dot
-                            \d*)     # followed by optional decimals
-                            ([E|e|d|D][+|-]?\d+)?  # optional exponents E+03, e-05, d0, D0
-                        ){2}         # I expect three float values
+                        {NUMBER_PATTERN}
+                        \s+
+                        {NUMBER_PATTERN}
+                        \s+
+                        {NUMBER_PATTERN}
                     )
                     |
                     \#
@@ -586,28 +565,25 @@ def _parse_cell_parameters(txt):
                 \s*              # A line only containing white space
              )
             [\n]                 # line break at the end
-        ){3}                     # I need exactly 3 vectors
+        ){{3}}                   # I need exactly 3 vectors
     )
     """, RE_FLAGS
     )
 
     cell_vector_regex = re.compile(
-        r"""
+        fr"""
         ^                        # Linestart
         [ \t]*                   # Optional white space
         (?P<x>                   # Get x
-            [\-|\+]? ( \d*[\.]\d+ | \d+[\.]?\d*)
-            ([E|e|d|D][+|-]?\d+)?
+            {NUMBER_PATTERN}
         )
         [ \t]+
         (?P<y>                   # Get y
-            [\-|\+]? (\d*[\.]\d+ | \d+[\.]?\d*)
-            ([E|e|d|D][+|-]?\d+)?
+            {NUMBER_PATTERN}
         )
         [ \t]+
         (?P<z>                   # Get z
-            [\-|\+]? (\d*[\.]\d+ | \d+[\.]?\d*)
-            ([E|e|d|D][+|-]?\d+)?
+            {NUMBER_PATTERN}
         )
         """, re.X | re.M
     )


Test patch:
diff --git a/tests/data/lattice_ibrav0_cell_parameters_int.in b/tests/data/lattice_ibrav0_cell_parameters_int.in
new file mode 100644
index 0000000..5e74490
--- /dev/null
+++ b/tests/data/lattice_ibrav0_cell_parameters_int.in
@@ -0,0 +1,15 @@
+&control
+ calculation = 'scf'
+/
+&system
+ ibrav = 0
+/
+ATOMIC_SPECIES
+ Na  22.99 Na.upf
+ATOMIC_POSITIONS crystal
+ Na   0.0  0.0  0.0
+CELL_PARAMETERS angstrom
+ 5.7 0 0
+ 0 5.7 0
+ 0 0 5.7
+K_POINTS gamma
diff --git a/tests/data/ref/lattice_ibrav0_cell_parameters_int.json b/tests/data/ref/lattice_ibrav0_cell_parameters_int.json
new file mode 100644
index 0000000..433b07a
--- /dev/null
+++ b/tests/data/ref/lattice_ibrav0_cell_parameters_int.json
@@ -0,0 +1,88 @@
+{
+  "atomic_positions": {
+    "fixed_coords": [
+      [
+        false,
+        false,
+        false
+      ]
+    ],
+    "names": [
+      "Na"
+    ],
+    "positions": [
+      [
+        0.0,
+        0.0,
+        0.0
+      ]
+    ],
+    "units": "crystal"
+  },
+  "atomic_species": {
+    "masses": [
+      22.99
+    ],
+    "names": [
+      "Na"
+    ],
+    "pseudo_file_names": [
+      "Na.upf"
+    ]
+  },
+  "cell": [
+    [
+      5.7,
+      0.0,
+      0.0
+    ],
+    [
+      0.0,
+      5.7,
+      0.0
+    ],
+    [
+      0.0,
+      0.0,
+      5.7
+    ]
+  ],
+  "cell_parameters": {
+    "cell": [
+      [
+        5.7,
+        0.0,
+        0.0
+      ],
+      [
+        0.0,
+        5.7,
+        0.0
+      ],
+      [
+        0.0,
+        0.0,
+        5.7
+      ]
+    ],
+    "units": "angstrom"
+  },
+  "k_points": {
+    "type": "gamma"
+  },
+  "namelists": {
+    "CONTROL": {
+      "calculation": "scf"
+    },
+    "SYSTEM": {
+      "ibrav": 0
+    }
+  },
+  "positions_angstrom": [
+    [
+      0.0,
+      0.0,
+      0.0
+    ]
+  ]
+}
diff --git a/tests/test_parsers.py b/tests/test_parsers.py
index 9081d88..119072c 100755
--- a/tests/test_parsers.py
+++ b/tests/test_parsers.py
@@ -222,6 +222,9 @@ def test_lattice_ibrav0_cell_parameters_ang(self):
     def test_lattice_ibrav0_cell_parameters_celldm(self):
         self.singletest(label='lattice_ibrav0_cell_parameters_celldm')
 
+    def test_lattice_ibrav0_cell_parameters_int(self):
+        self.singletest(label='lattice_ibrav0_cell_parameters_int')
+
     def test_lattice_ibrav1(self):
         self.singletest(label='lattice_ibrav1')