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

Add support for loading flattened meta_tags.csv #1212

Closed
S-aiueo32 opened this issue Mar 22, 2020 · 3 comments · Fixed by #1271
Closed

Add support for loading flattened meta_tags.csv #1212

S-aiueo32 opened this issue Mar 22, 2020 · 3 comments · Fixed by #1271
Assignees
Labels
feature Is an improvement or enhancement help wanted Open to be worked on
Milestone

Comments

@S-aiueo32
Copy link
Contributor

S-aiueo32 commented Mar 22, 2020

🚀 Feature

Motivation

PL+TensorBoard can log hierarchical dict after #1152, however, meta_tags.csv has been disabled by the change.

Pitch

  • Make meta_tags.csv back to a hierarchical dict based on their delimiter.

Alternatives

62de794

Additional context

  1. We can consider the deprecation of meta_tags.csv, then adopt config.yaml.
  2. We can interpret primitive-type parameters through the files, so it is needed to rethink parameter sanitization or update docs.
@S-aiueo32 S-aiueo32 added feature Is an improvement or enhancement help wanted Open to be worked on labels Mar 22, 2020
@S-aiueo32 S-aiueo32 changed the title Add support for meta Add support for hierarchical meta_tags.csv Mar 22, 2020
@S-aiueo32 S-aiueo32 changed the title Add support for hierarchical meta_tags.csv Add support for loading flattened meta_tags.csv Mar 22, 2020
@williamFalcon
Copy link
Contributor

can you elaborate on what the .yaml would look like? i kind of like the idea.
@luiscape

@S-aiueo32
Copy link
Contributor Author

@williamFalcon thank you for replying.

Roughly, I plan to make the following change for tags_csv replacement with yaml:

  1. Replace here with yaml.dump().
    import yaml
    
    with open("config.yaml", "w") as wf:
        yaml.dump(self.tags, wf)
  2. Create load_hparams_from_yaml like here
    import yaml
    
    def load_hparams_from_yaml(config_yaml: str) -> Dict[str, Any]:
        if not os.path.isfile(config_yaml):
            log.warning(f'Missing Tags: {config_yaml}.')
            return {}
    
        with open(config_yaml) as f:
           tags = yaml.load(f, Loader=yaml.SafeLoader)
    
        return tags

@Borda
Copy link
Member

Borda commented Mar 27, 2020

I very much support replacing CSV by YAML it is much easier to read and handle (also comment)

@Borda Borda added this to the 0.7.2 milestone Mar 27, 2020
@Borda Borda modified the milestones: 0.7.2, 0.7.3 Apr 8, 2020
@Borda Borda modified the milestones: 0.7.4, 0.7.5 Apr 26, 2020
@Borda Borda modified the milestones: 0.7.6, 0.8.0 May 12, 2020
@Borda Borda modified the milestones: 0.8.0, 0.7.6 May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement help wanted Open to be worked on
Projects
None yet
3 participants