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

nested expansion; #74 #75

Merged
merged 6 commits into from
Feb 16, 2021
Merged

nested expansion; #74 #75

merged 6 commits into from
Feb 16, 2021

Conversation

vreuter
Copy link
Member

@vreuter vreuter commented Feb 7, 2021

If I understood correctly, I think now I see the expected/desired behavior that wasn't seen for me on attmap dev and yacman dev, which motivated #74

In [1]: from yacman import YacAttMap

In [2]: y=YacAttMap(filepath="/Users/vince/temp.yaml")

In [3]: y.database.port
Out[3]: '5432'

In [4]: y.port
Out[4]: '5432'

In [5]: y.to_dict(expand=True)["database"]["port"]
Out[5]: '5432'

In [6]: y.to_dict(expand=True)["database"]
Out[6]: 
{'name': 'pipestat-test',
 'user': 'postgres',
 'password': 'pipestat-password',
 'host': 'localhost',
 'port': '5432'}

In [7]: cat /Users/vince/temp.yaml
name: test
record_identifier: sample1
schema_path: sample_output_schema.yaml
port: $PRT
database:
  name: pipestat-test
  user: postgres
  password: pipestat-password
  host: localhost
  port: $PRT

@vreuter vreuter requested review from nsheff and stolarczyk February 7, 2021 19:37
@vreuter
Copy link
Member Author

vreuter commented Feb 7, 2021

This is into dev so perhaps should precede merge of #72

Copy link
Member

@stolarczyk stolarczyk left a comment

Choose a reason for hiding this comment

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

Great, thanks for implementing this

This is into dev so perhaps should precede merge of #72

Agreed. Once it's merged, I'll run some tests with packages that depend on this to see if there are any obvious breaking effects.

@stolarczyk stolarczyk linked an issue Feb 7, 2021 that may be closed by this pull request
@stolarczyk
Copy link
Member

@nsheff should we merge this to dev to test for a while?

@nsheff
Copy link
Contributor

nsheff commented Feb 15, 2021

Sure.

@stolarczyk stolarczyk merged commit 3a65967 into pepkit:dev Feb 16, 2021
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.

recursive value expansion in PathExAttMap
3 participants