Skip to content
This repository has been archived by the owner on Oct 11, 2021. It is now read-only.

feat: add data structures for particle info #42

Closed
wants to merge 59 commits into from
Closed

feat: add data structures for particle info #42

wants to merge 59 commits into from

Conversation

redeboer
Copy link
Member

@redeboer redeboer commented Jun 2, 2020

Closes #40

This PR is part of the ongoing effort to make the codebase of the expertsystem more readible.

  • Cleans up and improves the existing Spin class
  • Introduces a ParticleDatabase (dict of Particle object) that is to replace the particle.DATABASE global
  • Since switching to ParticleDatabase requires refactoring almost the entire codebase, all old functionality has been moved to a namespace particle.deprecated, which will be gradually removed in follow-up PRs.
  • Run mypy over tests as well (but do not enforce type hints in the definitions there)
  • Compute finer test coverage with branch coverage.

(This PR has been rebased on #41)

@redeboer redeboer added Priority: High 🔨 Maintenance Refactoring that doesn't affect the interface labels Jun 2, 2020
@redeboer redeboer added this to the Improve handling of particle info milestone Jun 2, 2020
@redeboer redeboer requested a review from spflueger June 2, 2020 06:11
@redeboer redeboer self-assigned this Jun 2, 2020
@ghost
Copy link

ghost commented Jun 2, 2020

Congratulations 🎉. DeepCode analyzed your code in 0.366 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard
Edit DeepCode’s Configurations here

@codecov
Copy link

codecov bot commented Jun 2, 2020

Codecov Report

Merging #42 into master will decrease coverage by 10.40%.
The diff coverage is 91.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #42       +/-   ##
===========================================
- Coverage   85.34%   74.94%   -10.41%     
===========================================
  Files          16       20        +4     
  Lines        2395     3105      +710     
===========================================
+ Hits         2044     2327      +283     
- Misses        351      778      +427     
Impacted Files Coverage Δ
expertsystem/state/conservationrules.py 84.83% <76.92%> (+0.23%) ⬆️
expertsystem/state/particle.py 75.46% <87.09%> (-0.76%) ⬇️
expertsystem/particle.py 94.26% <94.26%> (ø)
expertsystem/__init__.py 50.00% <100.00%> (+2.94%) ⬆️
expertsystem/amplitude/canonicaldecay.py 96.96% <100.00%> (ø)
expertsystem/amplitude/helicitydecay.py 94.63% <100.00%> (-0.02%) ⬇️
expertsystem/state/propagation.py 83.11% <100.00%> (ø)
expertsystem/ui/default_settings.py 100.00% <100.00%> (ø)
expertsystem/ui/system_control.py 92.08% <100.00%> (-0.02%) ⬇️
expertsystem/solvers/constraint/version.py 100.00% <0.00%> (ø)
... and 4 more

@redeboer redeboer mentioned this pull request Jun 2, 2020
Copy link
Member

@spflueger spflueger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks quite good already. I made some small specific comments.
On a larger scale:

  • Can it be avoided having this deprecated version? I assume the amount of changes when replacing it are too big?
  • The ParticleDatabase code seems quite large and not DRY. I have to look at it more closely later. But it seems like the loading can be shortend. So just have a single load_from_dict method. The individual parsers for xml yml etc would just have to provide a to dict function. I personally would also just go with a single format as default (yml) and others would have to be implemented by the user himself if she/he prefers another format.

particle_list.yml Show resolved Hide resolved
particle_list.yml Show resolved Hide resolved
expertsystem/state/particle/__init__.py Outdated Show resolved Hide resolved
@redeboer
Copy link
Member Author

redeboer commented Jun 2, 2020

The DeepCode error is incorrect, because pyyaml and xmltodict don't work with binary mode. It seems the only way to ignore this issue is to put deepcode comments into the code. I don't like that, so I suggests giving up the deepcode checks -- it's too inflexible and and we already have sufficient checks through Codacy.

@redeboer redeboer marked this pull request as ready for review June 3, 2020 16:48
@redeboer
Copy link
Member Author

redeboer commented Jun 3, 2020

#42 (comment)
All serialisation functionality has been removed so that the classes in this are purely data objects. The construction of these data objects will be further addressed in #44.

@redeboer
Copy link
Member Author

redeboer commented Jun 3, 2020

Note that we may also want to add some tools that allow you to create a new particle instance based on another particle instance as template in Python code (so that you can for instance create duplicates of fake particles such as an e+i- collision at different energies or hypothetical particles). But as that functionality is not yet needed, I did not spend time on creating an interface for this yet.

--> #6

@redeboer redeboer changed the title feat: add data structures for state.particle module feat: add data structures for particle info Jun 3, 2020
@redeboer redeboer mentioned this pull request Jun 3, 2020
3 tasks
@redeboer
Copy link
Member Author

redeboer commented Jun 5, 2020

This PR will be split up into small PRs that address specific issues

@redeboer redeboer closed this Jun 5, 2020
@redeboer redeboer deleted the particle-data-structures branch June 5, 2020 09:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔨 Maintenance Refactoring that doesn't affect the interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encode particle info in the form of a class or data structure
2 participants