print version with `tensorboard --version` #2097

shashvatshahi1998 · 2019-04-06T06:41:39Z

Issue was related to this change
Added version flag in main
Screenshots of UI changes

Issue #2087

shashvatshahi1998 · 2019-04-06T06:42:48Z

@wchargin ,@nfelt can you please review these changes.

shashvatshahi1998 · 2019-04-06T08:55:17Z

@nfelt , @wchargin can you please tell me my mistake?

nfelt

Thanks for sending the PR!

nfelt · 2019-04-08T13:24:12Z

tensorboard/program.py

@@ -220,6 +220,9 @@ def main(self, ignored_argv=('',)):
      event_file = os.path.expanduser(self.flags.event_file)
      efi.inspect(self.flags.logdir, event_file, self.flags.tag)
      return 0
+    if self.flags.version:


You'll need to define the flag in core_plugin.py. See the --inspect flag definition for an example:

tensorboard/tensorboard/plugins/core/core_plugin.py

Lines 363 to 378 in dc71eee

parser.add_argument(

'--inspect',

action='store_true',

help='''\

Prints digests of event files to command line.

This is useful when no data is shown on TensorBoard, or the data shown

looks weird.

Must specify one of `logdir` or `event_file` flag.

Example usage:

`tensorboard --inspect --logdir mylogdir --tag loss`

See tensorboard/backend/event_processing/event_file_inspector.py for more info.\

''')

@shashvatshahi1998 if you're not working on this can i send pr for the same??

@nfelt Defined the flag in core_plugin.py

See #2100

@amanp592 I was also working on the issue man.

@amanp592 please let @shashvatshahi1998 finish working on this PR since they've already started it, and the functionality should all be submitted as a single PR.

If you're looking for a simple issue, you could pick up #1176 which is for ensuring that if --path_prefix is used, the URL printed out includes a trailing slash. The fix for that would be changing get_url() here to add a trailing slash to the URL if there isn't one already there:

tensorboard/tensorboard/program.py

Lines 519 to 527 in dc71eee

def get_url(self):

if self._auto_wildcard:

display_host = socket.gethostname()

else:

host = self._flags.host

display_host = (

'[%s]' % host if ':' in host and not host.startswith('[') else host)

return 'http://%s:%d%s' % (display_host, self.server_port,

self._flags.path_prefix)

A PR for that should also include a test to confirm the bug is fixed, which would go in

tensorboard/tensorboard/program_test.py

Line 43 in dc71eee

class WerkzeugServerTest(tf.test.TestCase):

@shashvatshahi1998 ohk cool sorry for that!

@nfelt Thankyou for refering me #1176 i'll look what can be done

@amanp592 no worries

shashvatshahi1998 · 2019-04-08T18:00:00Z

please @nfelt review it

shashvatshahi1998 · 2019-04-10T15:26:10Z

@nfelt please soomebody review this PR as its more than 4 days now .

stephanwlee · 2019-04-10T15:43:51Z

@shashvatshahi1998 Nick is currently out of office until end of today. Since this is not urgent feature, I would like to respect his time off. Would you mind waiting one more day?

Perhaps this is urgent to you, but I do not comprehend your requirements. I would appreciate it if you could you inform me why this is urgent. Thanks!

shashvatshahi1998 · 2019-04-10T16:32:08Z

@stephanwlee there is no urgency , I can wait for one more day. Basically I was not aware of the fact that @nfelt is not available.

shashvatshahi1998 · 2019-04-16T15:29:11Z

Please @nfelt, @stephanwlee somebody reviews this PR, it's like more than 10 days that I have opened it.

nfelt

Hi @shashvatshahi1998. We don't promise any particular turnaround time on PRs, and this particular addition is not urgent (not a bugfix or pressing feature). The team reviewing these has multiple priorities and we are not available 24/7 due to holidays, weekends, etc. I'd like to ask for a bit more patience going forward - multiple pings in a short time period (in this case, 5 within 24 hours last week) is not going to increase the chance that your PR gets reviewed, it will just spam the team when we have other work that is more pressing.

As for the PR contents: this looks good, but right now trying to run tensorboard --version will print an error about needing to supply --logdir or --db flags, which shouldn't be necessary to just print the version. Can you please fix this by modifying the check in fix_flags() here:

tensorboard/tensorboard/plugins/core/core_plugin.py

Line 467 in c38f4ec

if flags.inspect:

It should be updated to something like:

if flags.version:
  pass  # Don't validate flags if we're just printing the version.
elif flags.inspect:
  ...

shashvatshahi1998 · 2019-04-17T15:05:40Z

@nfelt i have done the changes which you asked me to do then why travis cl test is failing

nfelt · 2019-04-17T19:14:30Z

To fix the test, you'll need to edit core_plugin_test.py and add a version field to FakeFlags here:

tensorboard/tensorboard/plugins/core/core_plugin_test.py

Lines 46 to 58 in c38f4ec

    
           class FakeFlags(object): 
        
             def __init__( 
        
                 self, 
        
                 inspect, 
        
                 logdir='', 
        
                 event_file='', 
        
                 db='', 
        
                 path_prefix=''): 
        
               self.inspect = inspect 
        
               self.logdir = logdir 
        
               self.event_file = event_file 
        
               self.db = db 
        
               self.path_prefix = path_prefix

You should also add a test that you can pass just version and not get a flag error, by adding a line to this test:

tensorboard/tensorboard/plugins/core/core_plugin_test.py

Line 80 in c38f4ec

def testFlag(self):

E.g. add the line loader.fix_flags(FakeFlags(version=True)).

shashvatshahi1998 · 2019-04-17T20:30:04Z

@nfelt done with changes told by you.

nfelt · 2019-04-17T20:58:41Z

tensorboard/plugins/core/core_plugin_test.py

@@ -52,6 +52,7 @@ def __init__(
      db='',
      path_prefix=''):
    self.inspect = inspect
+    self.version = version


You also need to add version as a parameter to __init__().

nfelt · 2019-04-18T03:14:00Z

tensorboard/plugins/core/core_plugin_test.py

@@ -47,11 +47,13 @@ class FakeFlags(object):
  def __init__(
      self,
      inspect,
+      version,


This needs a default value of False, and so does inspect above.

shashvatshahi1998 · 2019-04-18T03:34:44Z

@nfelt done with changes asked by you

shashvatshahi1998 · 2019-04-18T03:44:16Z

@nfelt it has passed the checks now.

shashvatshahi1998 · 2019-04-18T09:41:16Z

@nfelt I think its complete now.

shashvatshahi1998 · 2019-04-19T14:11:37Z

@nfelt I want to work on more issues. Can you please give me some issues which are good for beginner?

nfelt · 2019-04-20T00:12:41Z

@shashvatshahi1998 you could take a look at #2121 to update our metaclass syntax to be compatible with Python 3.

shashvatshahi1998 added 4 commits April 1, 2019 11:08

Added datatype int64

7192eb8

solution for --version

c2c5a98

Resolved conflicts

032975f

Version issues

07493eb

Changes done

176d4f9

shashvatshahi1998 added 2 commits April 6, 2019 14:27

Changes 1

0e45e48

Changed

90c4736

nfelt reviewed Apr 8, 2019

View reviewed changes

Addded to core_plugin.py

c38f4ec

stephanwlee requested a review from nfelt April 10, 2019 15:40

stephanwlee mentioned this pull request Apr 10, 2019

Updated core_plugin.py #2100

Closed

nfelt suggested changes Apr 16, 2019

View reviewed changes

shashvatshahi1998 added 2 commits April 17, 2019 10:44

Modified core_plugin.py

d95e68f

Changes done

e1e40a7

Added to core_plugin_test.py

bff483d

nfelt reviewed Apr 17, 2019

View reviewed changes

Added version as parameter

93766f1

nfelt reviewed Apr 18, 2019

View reviewed changes

Default value provided

4f66094

nfelt approved these changes Apr 18, 2019

View reviewed changes

nfelt changed the title ~~tensorboard should support --version #2087~~ print version with tensorboard --version Apr 18, 2019

nfelt merged commit df4ac42 into tensorflow:master Apr 18, 2019

nfelt mentioned this pull request Apr 25, 2019

rename --version flag to --version_tb to avoid conflict #2153

Merged

nfelt mentioned this pull request May 15, 2019

tensorboard should support --version #2087

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

print version with `tensorboard --version` #2097

print version with `tensorboard --version` #2097

shashvatshahi1998 commented Apr 6, 2019

shashvatshahi1998 commented Apr 6, 2019

shashvatshahi1998 commented Apr 6, 2019 •

edited

Loading

nfelt left a comment

nfelt Apr 8, 2019

thealphacod3r Apr 8, 2019 •

edited

Loading

thealphacod3r Apr 8, 2019

shashvatshahi1998 Apr 8, 2019 •

edited

Loading

nfelt Apr 8, 2019

thealphacod3r Apr 8, 2019

thealphacod3r Apr 8, 2019

shashvatshahi1998 Apr 8, 2019

shashvatshahi1998 commented Apr 8, 2019

shashvatshahi1998 commented Apr 10, 2019

stephanwlee commented Apr 10, 2019

shashvatshahi1998 commented Apr 10, 2019

shashvatshahi1998 commented Apr 16, 2019

nfelt left a comment

shashvatshahi1998 commented Apr 17, 2019

nfelt commented Apr 17, 2019

shashvatshahi1998 commented Apr 17, 2019

nfelt Apr 17, 2019

nfelt Apr 18, 2019

shashvatshahi1998 commented Apr 18, 2019

shashvatshahi1998 commented Apr 18, 2019

shashvatshahi1998 commented Apr 18, 2019

shashvatshahi1998 commented Apr 19, 2019

nfelt commented Apr 20, 2019

	parser.add_argument(
	'--inspect',
	action='store_true',
	help='''\
	Prints digests of event files to command line.

	This is useful when no data is shown on TensorBoard, or the data shown
	looks weird.

	Must specify one of `logdir` or `event_file` flag.

	Example usage:
	`tensorboard --inspect --logdir mylogdir --tag loss`

	See tensorboard/backend/event_processing/event_file_inspector.py for more info.\
	''')

	def get_url(self):
	if self._auto_wildcard:
	display_host = socket.gethostname()
	else:
	host = self._flags.host
	display_host = (
	'[%s]' % host if ':' in host and not host.startswith('[') else host)
	return 'http://%s:%d%s' % (display_host, self.server_port,
	self._flags.path_prefix)

print version with tensorboard --version #2097

print version with tensorboard --version #2097

Conversation

shashvatshahi1998 commented Apr 6, 2019

shashvatshahi1998 commented Apr 6, 2019

shashvatshahi1998 commented Apr 6, 2019 • edited Loading

nfelt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thealphacod3r Apr 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashvatshahi1998 Apr 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashvatshahi1998 commented Apr 8, 2019

shashvatshahi1998 commented Apr 10, 2019

stephanwlee commented Apr 10, 2019

shashvatshahi1998 commented Apr 10, 2019

shashvatshahi1998 commented Apr 16, 2019

nfelt left a comment

Choose a reason for hiding this comment

shashvatshahi1998 commented Apr 17, 2019

nfelt commented Apr 17, 2019

shashvatshahi1998 commented Apr 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashvatshahi1998 commented Apr 18, 2019

shashvatshahi1998 commented Apr 18, 2019

shashvatshahi1998 commented Apr 18, 2019

shashvatshahi1998 commented Apr 19, 2019

nfelt commented Apr 20, 2019

print version with `tensorboard --version` #2097

print version with `tensorboard --version` #2097

shashvatshahi1998 commented Apr 6, 2019 •

edited

Loading

thealphacod3r Apr 8, 2019 •

edited

Loading

shashvatshahi1998 Apr 8, 2019 •

edited

Loading