-
Notifications
You must be signed in to change notification settings - Fork 32
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
Removing unused code #107
Removing unused code #107
Conversation
@psauvan I think we might need your expertise on parts of this PR as perhaps the code that is not run and removed by this PR is actually intended to run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes are fine and I don't expect changes during code run.
Please, consider my comment not to removing lines in no executed conditional blocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use forcing conditional statments with "logical operator + True/False " as a shortcut to line comment, when I want to keep lines in the code.
For now I would prefer to keep those lines. So instead of removing no executed blocks I prefer comment them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok I've kept the code in and just commented it out.
There is quite a lot of code commented out throughout the code base. We might consider moving that to issues at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as previous comment
@psauvan comment has been taken into account. Unreachable code that was there for future consideration has been changed to comments. |
* Add a github workflow that prevents PRs to main that is not comming from dev (#92) * Fix to position of additional cylindrical surface used to limit torus extent (#96) * camel case for all classes (#95) * camel case for all classes * corrected file imports for georeverse * Apply suggestions from code review variable back to snake case * variable back to snake case * back to snake case * back to previous var name * Snake case variables (PR 1 of several) (#98) * snake case for CellDefinitions file, tests pass * vars in snake_case 2 more files * working still * using is to compare with bool (#99) * Add comments to Geometry.py and booleanFunction.py files (#101) * Add comments to Geometry_GU.py * Add comments to booleanFunction.py * Add comments to Geometry_GU.py * Specific imports instead of wild card (#100) * improved import statements flake8 hints * removed extra line * solve issue #75 in getTransMatrix function (#103) * Changes in users initialization (#102) * Add a github workflow that prevents PRs to main that is not comming from dev (#92) (#93) * Add scripting interface Change to allow setting users parameters by either config file or geouned set method * minor fix * Changes in geounedClassCall.py --------- Co-authored-by: AlvaroCubi <55387701+AlvaroCubi@users.noreply.github.com> * Adding formatter to CI to keep code in PEP8 (#104) * added format checker * run black . locally * f strings instead of .format (#106) * replaced all .format with f strings * corrected f string conversion * Snake case variables (PR 2 of several) (#105) * snake case im more varibles * formatting --------- Co-authored-by: Patrick Sauvan <psauvan@ind.uned.es> * Quicker tests for ci (#109) * removing slowest two geometries * removing slow files from CI * format * added draft template (#97) * Removing unused code (#107) @psauvan comment has been taken into account. Unreachable code that was there for future consideration has been changed to comments. * if statement includes False so never runs code * if statement includes True so always run and never reaches else * line after return never reached * unused arg * unused arg * unused arg calling function * commented unused code as requested * formatting * format * Testing writing inputs for more MC codes (#108) * also writting openmc python, phits and serpent files * formatting * decreased lcoal run time and added all outFormats * black using same version as in pyproject * py3.11 in formatting ci * matched exactly black versions * simpler black pinning * format * Snake case functions (#115) * converted more functions * more snake case functions * more snake case function names * more snake case functions * more snake case functions * more snake case functions * formatting * formatting * formatting * formatting * converting function names to snake class * duplicate import openmc (#116) * Sphinx based docs with CI action to build and hosted on gh-pages (#119) * started rtd * testing docs action * update pip * corrected python version * added s to docs * publish every time * added static folder * skipping ci for non code files * changed to rerad the docs theme * started install section * added more sections * added api class * docs ci using conda * added -y for freecad * trying py 3.12 * mimic working conda ci * using rst so no markdown convertion needed * added github edit link * removed path prints * added readme link, badge * format * Add issue templates (#112) * Add a github workflow that prevents PRs to main that is not comming from dev (#92) (#93) * Create feature request template * improve documentation template * update feature request template --------- Co-authored-by: AlvaroCubi <55387701+AlvaroCubi@users.noreply.github.com> * Trigger docs build on push only (#126) * pushing triggers doics build * added if push * added missing = sign (#130) * added missing package (#135) * replaced print statements with log (#133) * replaced print statements with log * added logger * formatting * removed encoding arg * removed verbose as all gets logged * removed unused logger * removed , from logging cmd * sorted imports * added another tqdm loop * isort * format * Fixing logger commands (#137) * fixed multi comma log statements * capital letters for progress bar sections * formatted * removal of Options as global variable (#138) * instances of Options class passed by arg * updated usage to show how to use Options * format pep8 * added missing options arg * added missing args options * passing in options as arg * bug fix for compsolid * format * removal of Tolerances as global variable (#139) * passing tolerance as arg not global var * added tolerance example to usage section * one set of argument names * moved atr and types getting out of loop * format * passing missing options and tolerances * format * found bug hidden in try: except statement * using tolerance for more local varibles * formatting * Numerical format to normal class (#141) * replaced global with class passed by arg * format * Improving docs install section (#145) * added dev install instructions * added method section * testing the setting of all class arguments (#147) * Adding getters setters type checking to Options, Tolerances, NumericFormat class (#148) * added type ching setters * added low limit checking for reals * format * added type checkers for Tolerances * format * Add settings class (#149) * settings being passed through the code * all the attributes show in usage example * Adding export csg method (#150) * added export csg method * format * leaving comments in tests * Adding support for json config file (#152) * testing from json * returned file to pre PR status * returned file to init * returned file to init * temp fixing for serpent universe, issue raised * making use of stp file already on repo * Adding command line tool including documentation and testing (#155) * split usage into two sections * added minimal config from cli example to ci * improved cli docs * format * removed duplication between readme and docs (#156) * Moving code from __init__ file to core.py (#157) * moved core logic from init * format * Fixing issue 154 (#158) * added test that currently fails * fixed bug with freecad attribute usage * added missing arg * format * added missing args bug (#159) * added missing args * format * added description of solid being decomposed (#161) * Context managers for writing try 2 (#160) * Add a github workflow that prevents PRs to main that is not comming from dev (#92) (#93) * using context managers to write files * review comments 4/5 --------- Co-authored-by: AlvaroCubi <55387701+AlvaroCubi@users.noreply.github.com> * Creating folder if needed (#162) * added tests to check writing to folder * back to org * back to org * back to org * creating output folder prior to writing * isort on the imports * format * setting line length to 128 (#163) * Lower case folder names and files (#165) * renamed subfolders to lowercase * renamed files to lower case * openmc_py and openmc_xml lower case (#166) * openmc_py and openmc_xml lower case * removed outdated code comment * Added type checking on export csg (#167) * added input checking on export_csg * compacted same types * Update src/geouned/GEOUNED/core.py --------- Co-authored-by: Patrick Sauvan <psauvan@ind.uned.es> * Adding windows install instructions (#169) * added windows install instructions * improved install instructions --------- Co-authored-by: teade <37874718+teade@users.noreply.github.com> Co-authored-by: Jonathan Shimwell <drshimwell@gmail.com> Co-authored-by: Jonathan Shimwell <mail@jshimwell.com> Co-authored-by: Patrick Sauvan <psauvan@ind.uned.es> Co-authored-by: alberto743 <4104972+alberto743@users.noreply.github.com> Co-authored-by: Alex Valentine <40658938+alexvalentine94@users.noreply.github.com>
I spotted a few places where code is written but is not used because it is not reachable
This PR is just to show where the code is that is not reachable, I think these might be coding mistakes.
These might be accidents in the programming and we might want to use this PR to bring these bits of code back to being usable.
the
fileout
arg is passed in and not used, I think this is easy to correct with this PRPart is imported but not used. again easily corrected with this PR
if .. or False
never runs, perhaps this is a mistake? Careful review neededif .. or True
always runs so if it has an else block then the else block never runs, perhaps this is a mistake? Careful review needed