Fix/deprecate 'from_pandas' interface

[ischoegl]

pandas dataframes do not include information on boundaries and thus cannot
be used to recreate all information required to restore a simulation object.

Changes proposed in this pull request

• fix from_pandas keyword arguments
• deprecate from_pandas as it does not allow for a complete restoration of the object

Note that this API was used for a previous implementation of a pandas-based HDF export/import route, which has since been refactored.

If applicable, fill in the issue number this pull request is fixing

Closes #1235

Checklist

• The pull request includes a clear description of this code change
• Commit messages have short titles and reference relevant issues
• Build passes (scons build & scons test) and unit tests address code coverage
• Style & formatting of contributed code follows contributing guidelines
• The pull request is ready for review

[ischoegl]

@bryanwweber ... I'm responding outside of the feedback, as I believe there are some larger issues.

Fwiw, from_pandas is a vestigial method that is left over from the original HDF interface that built on pandas and was replaced by the current implementation between 2.4 and 2.5.1 (PR #840); the original HDF implementation was never released. Before #840, this function was an intermediate step of the HDF interface and was implicitly covered by the test suite. The only reason from_pandas still exists is that I overlooked it when removing the previous HDF incarnation.

I guess you're saying that the user should set up the FreeFlame and then pass the SolutionArray created from pandas to specifically set the state of the flame domain within the FreeFlame? I also see that this method is doing that, so why do we need to deprecate/remove this method?

While this is correct, I believe that having a dedicated function has the potential to create more confusion than actual benefit. Users will need to know some details about how these objects work, and it's not too much to ask to just write two extra lines in Python. Fwiw, CSV support is something that would be used in 95% of the cases, as I don't expect pandas to play a major role.

let's not shoot ourselves in the foot by removing things only to add them back later.

I think that what is done here should ultimately be handled by restore ... even if this will require some heavy lifting on the C++ backend. At this point we need to start thinking about how to create SolutionArray equivalents that can be used from other API's, so I don't want to spend too much time creating yet another Python-centric feature. I am mostly referring to CSV support here, which pandas would piggy-back on.

PS:

In principle, they can be composed of an arbitrary number of domains, I think; as a practical matter, the most convenient ones that we've implemented are composed of three domains

This is of course correct, but it's not used. There are comments about this in 1D Flame Development Ideas (or at least there were at some time); imho this is really just a tangential issue here.

[bryanwweber]

The only reason from_pandas still exists is that I overlooked it when removing the previous HDF incarnation.

Then I think we should just raise a NotImplementedError for now and come back to this after the release.

[decaluwe]

I think this is the correct approach for now. Only one suggestion I have: should the docstring lead with a notification that this feature is currently deprecated/disabled? Seems like that would be most pertinent, before somebody decides to read/parse the rest of the docstring.

[ischoegl]

@decaluwe ... fair point - docstring is updated per your suggestion.

[decaluwe]

Thanks, @ischoegl - LGTM.