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

pass labels argument through when reading chains #232

Merged
merged 8 commits into from
Jan 17, 2023

Conversation

AdamOrmondroyd
Copy link
Collaborator

I noticed that labels isn't passed along in the same way as columns when reading chains.

@codecov
Copy link

codecov bot commented Sep 16, 2022

Codecov Report

Merging #232 (d6ae643) into master (fffa42e) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #232   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines         2271      2276    +5     
=========================================
+ Hits          2271      2276    +5     
Impacted Files Coverage Δ
anesthetic/read/cobaya.py 100.00% <100.00%> (ø)
anesthetic/read/getdist.py 100.00% <100.00%> (ø)
anesthetic/read/multinest.py 100.00% <100.00%> (ø)
anesthetic/read/polychord.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@lukashergt lukashergt left a comment

Choose a reason for hiding this comment

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

Hi @Ormorod,

thanks so much for stress testing and contributing to anesthetic!

  • Could we go for a consistent use of '.' rather than "." for the dictionary keys? This probably isn't perfectly consistent throughout anesthetic, but it's good to clean these things up when they come up...
  • Could you do the changes also for anesthetic/read/getdist.py, please?

@AdamOrmondroyd
Copy link
Collaborator Author

I can, guess I should get used to it (I've always preferred ", as I find myself much more often needing an apostrophe than quotation marks in a string)

@williamjameshandley should I switch to ' in other PRs? Thinking of the **kwargs PolyChord interface in particular

@lukashergt
Copy link
Collaborator

I can, guess I should get used to it (I've always preferred ", as I find myself much more often needing an apostrophe than quotation marks in a string)

There is no rule about this, PEP8 takes a mostly neutral stance, except saying to avoid escaping with backslashes. However, I think there is somewhat of a community consensus (take e.g. the votes on this stackexchange answer) which I try to follow where

  • double quotes " are used for text, e.g. in print statements, warnings or error messages (which accommodates the use of apostrophes...),
  • single quotes ' are used for single characters, dictionary keywords or other identifiers such as bins='auto' in histograms.

So that is what I'd suggest to go with. While I am open to other suggestions, I am strongly in favour of some form of consistency, so seeing 'columns' and "labels" in successive rows seems wrong to me...

anesthetic/read/polychord.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@lukashergt lukashergt left a comment

Choose a reason for hiding this comment

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

Thanks for this @Ormorod, please feel free to squash and merge.

@AdamOrmondroyd AdamOrmondroyd merged commit 9f57446 into handley-lab:master Jan 17, 2023
@AdamOrmondroyd AdamOrmondroyd deleted the labels branch January 17, 2023 16:46
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.

3 participants