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

Removing Dead, Deprecated, and Unreachable Code #975

Closed
goldmanm opened this issue Mar 28, 2017 · 17 comments · Fixed by #2516
Closed

Removing Dead, Deprecated, and Unreachable Code #975

goldmanm opened this issue Mar 28, 2017 · 17 comments · Fixed by #2516
Labels
Python 3.11 Transition PRs and Issues related to transitioning from Python 3.7 to 3.11 Type: Deprecation

Comments

@goldmanm
Copy link
Contributor

I am curious if we want to deprecate any functionality in RMG. I am thinking that not-often-used and poorly understood functionalities could be removed to allow easier bug checking, code support, and development. We may also be able to just cut some redundant methods. One example I came across would be to remove the method rmgpy/molecule/molecule/Bond/isSpecificCaseOf and integrate its code into rmgpy/molecule/molecule/Bond/equivalent.

If people think it may be worthwhile to have a list of methods to cut, we may want to have a warning placed in docStrings, similar to how Cantera placed some in their code to warn users.

@goldmanm
Copy link
Contributor Author

#1021 mentions some to get rid of

@goldmanm
Copy link
Contributor Author

goldmanm commented Jun 27, 2017

I couldn't find a place where the attribute Reaction.label is clearly assigned and utilized. Possibly we could deprecate it.

The Reaction.__str__ doesn't even use the label attribute.

@goldmanm
Copy link
Contributor Author

I just thought we might want to deprecate the restart file functionality. It didn't seem like it was working over 2 years ago, and as of now, it doesn't seem to be usable either (due to long saving times).

@alongd
Copy link
Member

alongd commented Jun 29, 2017

Does anyone use RMG-Java?
Perhaps we could depreciate the duplicate code for dealing with this older format?
I think that leaving just the import scrips is enough.

@mliu49
Copy link
Contributor

mliu49 commented Jun 29, 2017

I agree with deprecating RMG-Java methods. See #1021 for additional discussion related to this.

@mliu49
Copy link
Contributor

mliu49 commented Jun 29, 2017

As far as I'm aware, we don't have a existing deprecation process. It might be good to start one though. I think for typical software projects, deprecation means that a feature is no longer supported or recommended for use. Complete removal of the feature is done later on a fixed timeline.

This might be difficult for us though, since we don't have a fixed release schedule. A possible plan for us might be to add the deprecation warning in one release, then remove the method in the next release. Unless we start making small releases more often, in which case we could have a couple releases between deprecation and removal.

@alongd
Copy link
Member

alongd commented Jun 29, 2017

I was too fast to vote for removing on #1021. I agree that we should develop a deprecation process.

@goldmanm
Copy link
Contributor Author

We may also want to deprecate the non-web versions of scripts like diffModels, and only support one type of output for them.

@alongd
Copy link
Member

alongd commented Feb 8, 2018

Perhaps allowSingletO2 option in the input file can now be depreciated.
If O=O is generated, an unidirectional template (ReactionMechanismGenerator/RMG-database#239) converting it to [O][O] is applied.

@alongd
Copy link
Member

alongd commented Feb 15, 2018

We could also depreciate the saveRestartPeriod now that automatic seed generation works
Edit: I see @goldmanm already mentioned that above. It might be time to remove it.

@goldmanm
Copy link
Contributor Author

I'd add group additivity methods in 'family.py' to the list of deprecatable methods.

@alongd
Copy link
Member

alongd commented Jul 27, 2018

We could deprecate the FAME (used in RMG-Java) to CanTherm/Arkane file type converter.
(Has anyone ever seen a FAME input file lately?)

@goldmanm
Copy link
Contributor Author

I wanted to add a few more things to this thread:

  • It seems like there are two methods to calculate collision rate. One is in reaction.py. The other one is in pdep/configuration.pyx. Each uses completely separate code to calculate the rate. We should consolidate these.
  • Some methods in network.py, like solveFullME and solveReducedME, are not used in the rest of the program.

@alongd
Copy link
Member

alongd commented Jan 4, 2020

Searching for DeprecationWarning in the repository brings up many

warnings.warn("X is no longer supported and may be"
              " removed in version 2.3.", DeprecationWarning)

results.

Shall we go ahead and remove these functions, given that v 3.0.0 was just released?

@JacksonBurns
Copy link
Contributor

Here is a list of methods which should (allegedly) be deleted: link

@JacksonBurns JacksonBurns changed the title any methods to deprecate? Removing Dead, Deprecated, and Unreachable Code May 12, 2023
@JacksonBurns JacksonBurns pinned this issue May 12, 2023
@JacksonBurns
Copy link
Contributor

The ci-report-status step of the CI can also be removed since the target for required CI was set to both the ubuntu and mac runners.

@JacksonBurns
Copy link
Contributor

The ci-report-status step of the CI can also be removed since the target for required CI was set to both the ubuntu and mac runners.

Done in #2448

@JacksonBurns JacksonBurns added the Python 3.11 Transition PRs and Issues related to transitioning from Python 3.7 to 3.11 label Jun 8, 2023
@JacksonBurns JacksonBurns mentioned this issue Jul 22, 2023
@JacksonBurns JacksonBurns linked a pull request Jul 22, 2023 that will close this issue
@JacksonBurns JacksonBurns linked a pull request Aug 2, 2023 that will close this issue
@JacksonBurns JacksonBurns unpinned this issue Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Python 3.11 Transition PRs and Issues related to transitioning from Python 3.7 to 3.11 Type: Deprecation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants