@davis8dd think you'd be interested?

While pondering this problem, it's always wise to click around on this unbelievable hexagon page. They even have an implementation guide in many languages, including python!

That said, the basic requirements of this kind of system would be to convert various 2-D geometrical grids to and from a compact ASCII-based representation losslessly, including:

• Cartesian geometry
• Hexagon geometry (both flats up and points up)
• RZ geometry (may be basically identical to Cartesian) (bonus points, not required)

The mapping is preferred to be whitespace delimited but multi-character. We sometimes back things like (5,4) as well as F as well as GT into indices.

Note that this messes with brains a bit because while matrix nomenclature usually have an 'origin' at the top left, spatial coordinates usually have the origin in the bottom left. So we have to decide in advance which one we want. In other words, we have to pick a specific mapping style for each and make a diagram describing it.

Example:

"""
A B
C D
"""

might lead to

{ 
    (0,0): "C",
    (0,1): "D",
    (1,0): "A", 
    (1,1): "B"
}

or 4 or more other possible mappings. Thus, we should explicitly specify these in advance for each geometry option with a graphical example and then once we all agree on what the mappings should look like, then we can implement. That's effectively what we have done in test_asciimaps.py to a degree but it could be better.

Question -- I'm considering using dataclasses as an implementation detail (since __post_init__ is nice), but it is only available from 3.7 onwards. I suspect that's okay, since the latest release of ARMI appears to be tested against 3.7, 3.8, and 3.9 but not 3.6: https://github.com/terrapower/armi/runs/4267083946?check_suite_focus=true

However, since setup.py says >=3.6, I thought it would be good to check first. (there's a typo there too -- "s/requres/requires")

So can I assume the user has python >=3.7 ?

I think it's a fair assumption that most users have Python 3.7+ internally at TerraPower, but I cannot speak for all external users. Right now the installation docs do state Python 3.6+ even though the setup.py is broken.

It seems, then, that either release v0.2 should be tested on python 3.6 (since it seems like it currently isn't), or that setup.py and the release doc should be updated to say it requires >=3.7 ?

I think dropping python 3.6 support might not be a terrible idea -- if someone is using python 3.6, they could depend on the previous release. Since release v0.2 was only 5 days ago, it seems unlikely that anyone is relying on it yet. This could be a good opportunity to drop python 3.6 support -- otherwise you might have to wait till the next major release to do so :(

The reason why I'm hoping that python 3.6 support can be dropped is that, besides dataclasses, python 3.7 also brings types and generics, which would make refactorings and cleanups so much easier. Perhaps some subclasses of abc.ABC can also be simplified.

@john-science - maybe we can hot patch the v0.2 release to enforce 3.7+? I don't see a reason why we need to support 3.6 at this time.

@jakehader @scottyak My preference is to only support Python 3.7-3.9, as those are really the only versions that we test for seriously. And while I'm generally a fan of supporting a wide breadth of versions for an importable library... I don't know, I think for a scientific modeling framework I'm more in favor of constraining the versions to be better tested.

Let me ask around, and see if anyone depends on 3.6.

@jakehader @scottyak It looks like no one is using Python 3.6 anyway. Nick is hesitant to drop support for 3.10, so I think saying we support Python 3.7+ is a pretty safe bet for everyone.

@scottyak, the 3.6 requirement has been removed in #491.

@sdow1ll Is taking this ticket!

Thanks Silba!

(I just assigned it to myself so no one else would grab it.)

Sorry for leaving this one assigned for so long!

Hi @john-science I have a working Jupyter Notebook of the asciimaps.py file that basically has the converted functions from the asciimap class, but one thing that I got stuck on was that I noticed at the end of the asciimaps.py file, there is a code block of the following:

def asciiMapFromGeomAndSym(geomType: str, symmetry: str):
    """Get a ascii map class from a geometry type."""
    from armi.reactor import geometry
    
    MAP_FROM_GEOM = {
        (geometry.HEX, geometry.THIRD_CORE): AsciiMapHexThird,
        (geometry.HEX, geometry.FULL_CORE): AsciiMapHexFullTipsUp,
        (geometry.CARTESIAN, None): AsciiMapCartesian,
        (geometry.CARTESIAN, geometry.FULL_CORE): AsciiMapCartesian,
    }
return MAP_FROM_GEOM[(geomType, symmetry)]

Is this something that I should try to edit as well? When looking at this, I notice that the MAP_FROM_GEOM dictionary relies on the original classes. Are the functions I wrote basically an additional feature to this file, and that I should not touch the following code block above?

The only two references to that function are here:

So, refactoring those two use-cases will be necessary to do your refactoring of the asciimaps utils.

(Sorry for the long delay.)