sunpy / sunpy

SunPy - Python for Solar Physics
http://www.sunpy.org
BSD 2-Clause "Simplified" License
919 stars 590 forks source link

Convert models to something usable #2940

Open dpshelio opened 5 years ago

dpshelio commented 5 years ago

Description

sunpy/sun/models.py doesn't provide a proper way to use it. Either you know what it gives or you don't. At least this should be as part of a function that can be called and tested(! - so we can discover if we change some values/units/... by mistake).

They should return the appropriate tables for each.

Moving forward they could become some kind of attributes of a sun class or something else like the constants... But we should not fill our users variables with intermediate steps as currently is.

Additional context

Related with #2933 and #2936

Cadair commented 5 years ago

But we should not fill our users variables with intermediate steps as currently is.

I am not sure I follow you on this?

To your wider point, I don't really know what the usecase for this is, so I don't know what to suggest as improvements. We could try expose them as astropy.modeling.Model instances if the default usecase is to interpolate the values?

dpshelio commented 5 years ago

But we should not fill our users variables with intermediate steps as currently is.

I am not sure I follow you on this?

At the moment if you import that file you also get all these _time, _tradius, variables. If you know that you only want to import a table... then you get it, but if you don't know... then you get all these variables that are useless.

We could try expose them as astropy.modeling.Model instances if the default usecase is to interpolate the values?

Maybe one the user case is to interpolate, and that's a possibility. Another is just to visualise it... and that would work too.

Cadair commented 5 years ago

At the moment if you import that file you also get all these _time, _tradius, variables.

I mean they are hidden, it's not really the end of the world. but :woman_shrugging:

wtbarnes commented 2 years ago

@DanRyanIrish do you think that these belong in your theorized "models" package?