Skip to content

Conversation

reykboerner
Copy link
Contributor

We are using referrenced_sciml_prob downstream but it is currently neither exported nor declared public. Also, it would be nice to switch to a correct spelling of referenced. Maybe this can be solved with an alias for now and then the old spelling can become deprecated in the future?

Copy link
Member

@Datseris Datseris left a comment

Choose a reason for hiding this comment

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

  • Version bump is missing
  • Correct spelling should be the spelling used everywhere with a mass replace. The wrong spelling should be the deprecation and it should be in the file deprecated.jl at top-level (if it doens't exist we make one now).

"""
referrenced_sciml_prob(ds::Dynamical system)
Return `ds.integ.sol.prob` if there is a referenced model.
Copy link
Member

Choose a reason for hiding this comment

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

You should say what it returns instead of the field that it returns. Internal fields are not public.

# Simply extend the `referrenced_sciml_prob` and you have symbolic indexing support!
import SymbolicIndexingInterface
"""
referrenced_sciml_prob(ds::Dynamical system)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
referrenced_sciml_prob(ds::Dynamical system)
referrenced_sciml_prob(ds::DynamicalSystem)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants