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

Fixed bug in print_decay_modes where decays with equal BFs were skipped #452

Merged
merged 6 commits into from
Aug 9, 2024

Conversation

manuelfs
Copy link
Contributor

This PR fixes #451.

I also reduced the number of decimals printed to 5 or 7.

@eduardo-rodrigues
Copy link
Member

eduardo-rodrigues commented Jul 29, 2024

Hi @manuelfs, thank you for the fix. I'm sure you tested it but one still needs to add a test to ensure all is fine (just use my minimal reproducer #451 (comment)). Will you take care of that? And also of fixing the pre-commit failure? (I'm not sure I will have time before I depart on holidays. Otherwise I will get back to this upon return.)

@manuelfs
Copy link
Contributor Author

manuelfs commented Aug 2, 2024

There was already a "test" of the print_decay_modes function, though it cannot fail as it only prints. I added a new function to print a more complete .dec file where you would have seen the bug, but I don't think it would be easy to assert a condition for this printing function.

I don't know how to handle the error in the pre-commit check, I don't see that in my system. Also, I get a super long traceback when I try to run nox...

...
nox > pylint src
PYLINTHOME is now '/Users/manuelf/Library/Caches/pylint' but obsolescent '/Users/manuelf/.pylint.d' is found; you can safely remove the latter
Exception on node <ImportFrom l.5 at 0x10e282210> in file '/Users/manuelf/code/decaylanguage/src/decaylanguage/_version.py'
Traceback (most recent call last):
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/pylint/checkers/imports.py", line 798, in _get_imported_module
    return importnode.do_import_module(modname)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/astroid/nodes/_base_nodes.py", line 146, in do_import_module
    return mymodule.import_module(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/astroid/nodes/scoped_nodes/scoped_nodes.py", line 527, in import_module
    return AstroidManager().ast_from_module_name(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/astroid/manager.py", line 232, in ast_from_module_name
    return self.ast_from_file(found_spec.location, modname, fallback=False)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/astroid/manager.py", line 124, in ast_from_file
    return AstroidBuilder(self).file_build(filepath, modname)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/astroid/builder.py", line 144, in file_build
    module, builder = self._data_build(data, modname, path)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/astroid/builder.py", line 204, in _data_build
    module = builder.visit_module(node, modname, node_file, package)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/astroid/rebuilder.py", line 254, in visit_module
    [self.visit(child, newnode) for child in node.body],
     ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/manuelf/code/decaylanguage/.nox/pylint/lib/python3.12/site-packages/astroid/rebuilder.py", line 603, in visit
    visit_method = getattr(self, visit_name)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'TreeRebuilder' object has no attribute 'visit_typealias'
...
  File "/usr/local/Cellar/python@3.12/3.12.2_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/concurrent/futures/_base.py", line 449, in result
    return self.__get_result()
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/Cellar/python@3.12/3.12.2_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/concurrent/futures/_base.py", line 401, in __get_result
    raise self._exception
astroid.exceptions.AstroidError
nox > Command pylint src failed with exit code 1
nox > Session pylint failed.
nox > Running session tests-3.7

@eduardo-rodrigues
Copy link
Member

Hi @manuelfs. I'm back and am going to take a look ...

Can you commit your custom test file to get rid of the error

test_custom_model_name

FileNotFoundError: '../data/test_custom_decay_model.dec'!

and actually test things?

I don't use nox (yet), sorry.

@eduardo-rodrigues
Copy link
Member

Never mind, the file is there !

@eduardo-rodrigues
Copy link
Member

Let me know if you are also happy with this new version of the PR.

@eduardo-rodrigues
Copy link
Member

The update

-        "MyRho0": {"mass": 0.8, "width": 149.1},
+        "MyRho0": {"mass": 0.8, "width": 147.4},

hints at a bug upon starring a bit more at why this is updated. As I would like to address that asap and I'm unsure if you are away on holidays, I will go ahead and merge this. Kindly follow-up if you see a need.

Thank you for this contribution.

@eduardo-rodrigues
Copy link
Member

@all-contributors please add @manuelfs for code

@eduardo-rodrigues eduardo-rodrigues merged commit 12fa6b3 into scikit-hep:master Aug 9, 2024
12 checks passed
Copy link
Contributor

@eduardo-rodrigues

I've put up a pull request to add @manuelfs! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MyDecFileParser.print_decay_modes ommits decays if BF is equal to others
2 participants