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

class hierarchy docs and naming convention ideas #38

Closed
nsheff opened this issue May 15, 2019 · 5 comments

Comments

@nsheff
Copy link
Contributor

commented May 15, 2019

it would be nice to add to the docs something like this to codify the inheritance of the maps:

- AttMap
  - OrdAttMap
    - PathExAttMap
      - AttMapEcho

At some point we may have multiple subclasses under one of them, and this would accommodate that nicely.

Also, I was wondering what you think about making a naming convention, which is: all subclasses of Attmap are named in the same way, which is: ModifierAttMap, where Modifier is the lowest attribute in the class hierarchy above. This will avoid some of the confusion that we were facing with OrdPathExAttMap, which can get even hairy with 4-5 levels down (like EchoOrdPathExAttMap), etc. So we'd just say an "EchoAttMap" would fit this standard.

Our classes would be: AttMap, OrdAttMap, PathExAttMap, EchoAttMap at the moment. I'm working on another that will be:

class YacAttMap(attmap.OrdAttMap):

provided by yacman, which will add the yaml reading/writing stuff.

@vreuter vreuter modified the milestones: 0.11, 0.12 May 15, 2019

@vreuter

This comment has been minimized.

Copy link
Member

commented May 16, 2019

Whenever you're comfortable with me completely removing AttMapEcho, I'm all in.

@vreuter vreuter removed this from the 0.12 milestone May 16, 2019

@nsheff

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

can we:

  1. Add EchoAttMap and start using it
  2. Alias AttMapEcho until we're ready to do the next round of updates on all the tools currently depending on AttMapEcho
  3. Remove AttMapEcho after those are released.

?

this seems like the most prudent approach.

@vreuter

This comment has been minimized.

Copy link
Member

commented May 16, 2019

What's the upside to adding EchoAttMap and deliberately introducing the need for dual naming, rather than naming it AttMapEcho wherever you want to use it anew, and then doing all the changes in one sweep?

@nsheff

This comment has been minimized.

Copy link
Contributor Author

commented May 16, 2019

that I don't have to do all the work of changes in one sweep. development cycle continues normally.

really, to me, this seems like the only way to do it. I can't comprehend the approach of going through the effort to re-release all downstream stuff immediately.... it just doesn't make sense to me to do all that extra work. it's so much easier to just release 1 version of attmap that has 2 ways to do it, and then remove after the normal release cycles of all other software are complete...

vreuter added a commit that referenced this issue May 16, 2019

@vreuter vreuter closed this in 177bd4b May 17, 2019

@nsheff

This comment has been minimized.

Copy link
Contributor Author

commented May 17, 2019

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.