Skip to content

Remove the demoify function #490

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

Merged
merged 1 commit into from
Sep 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion tensorboard/components/tf_backend/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ limitations under the License.
import {compareTagNames} from '../vz-sorting/sorting.js';
import {RequestManager} from './requestManager.js';
import {getRouter} from './router.js';
import {demoify, queryEncoder} from './urlPathHelpers.js';

export type RunToTag = {
[run: string]: string[];
Expand Down
12 changes: 3 additions & 9 deletions tensorboard/components/tf_backend/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/

import {demoify, queryEncoder} from './urlPathHelpers.js';
import {queryEncoder} from './urlPathHelpers.js';

export type RunTagUrlFn = (tag: string, run: string) => string;

Expand All @@ -35,19 +35,13 @@ export interface Router {
* @param demoMode {boolean} Whether to modify urls for filesystem demo usage.
*/
export function createRouter(dataDir = 'data', demoMode = false): Router {
var clean = demoMode ? demoify : (x) => x;
if (dataDir[dataDir.length - 1] === '/') {
dataDir = dataDir.slice(0, dataDir.length - 1);
}
function standardRoute(route: string, demoExtension = '.json'):
((tag: string, run: string) => string) {
return function(tag: string, run: string): string {
var url =
dataDir + '/' + route + clean(queryEncoder({tag: tag, run: run}));
if (demoMode) {
url += demoExtension;
}
return url;
return dataDir + '/' + route + queryEncoder({tag: tag, run: run});
};
}
function pluginRoute(pluginName: string, route: string): string {
Expand All @@ -56,7 +50,7 @@ export function createRouter(dataDir = 'data', demoMode = false): Router {
function pluginRunTagRoute(pluginName: string, route: string):
((tag: string, run: string) => string) {
const base = pluginRoute(pluginName, route);
return (tag, run) => base + clean(queryEncoder({tag, run}));
return (tag, run) => base + queryEncoder({tag, run});
}
return {
logdir: () => dataDir + '/logdir',
Expand Down
19 changes: 4 additions & 15 deletions tensorboard/components/tf_backend/test/backendTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,13 @@ limitations under the License.
import {filterTags, getRuns, getTags, RunToTag, TYPES} from '../backend.js';
import {RequestManager} from '../requestManager.js';
import {createRouter, setRouter} from '../router.js';
import {BAD_CHARACTERS, demoify, queryEncoder} from '../urlPathHelpers.js';
import {queryEncoder} from '../urlPathHelpers.js';

describe('urlPathHelpers', () => {
it('demoify works as expected', () => {
const demoified = demoify(BAD_CHARACTERS);
let allClean = '';
for (let i = 0; i < BAD_CHARACTERS.length; i++) {
allClean += '_';
}
chai.assert.equal(demoified, allClean, 'cleaning the BAD_CHARACTERS works');
chai.assert.equal(demoify('foozod'), 'foozod', 'doesnt change safe string');
chai.assert.equal(demoify('foo zod (2)'), 'foo_zod__2_', 'simple case');
});

it('queryEncoder works with demoify on spaces and parens', () => {
it('queryEncoder works on spaces and parens', () => {
const params = {foo: 'something with spaces and (parens)'};
const actual = demoify(queryEncoder(params));
const expected = '_foo_something_with_spaces_and__28parens_29';
const actual = queryEncoder(params);
const expected = 'something+with+spaces+and+%28parens%29';
chai.assert.equal(actual, expected);
});
});
Expand Down
12 changes: 0 additions & 12 deletions tensorboard/components/tf_backend/urlPathHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,6 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
export const BAD_CHARACTERS = '#%&{}\\/<>*? $!\'":@+`|=() ';
/** Cleanup a url so that it can be loaded from a filesystem. */
export function demoify(s) {
// for consistency with python's urllib.urlencode
s = s.replace(new RegExp('%20', 'g'), '+');
for (let i = 0; i < BAD_CHARACTERS.length; i++) {
const c = BAD_CHARACTERS[i];
s = s.replace(new RegExp('\\' + c, 'g'), '_');
}
return s;
}

export function queryEncoder(params?: any): string {
// It's important that the keys be sorted, so we always grab the right file
// if we are talking to the backend generated by serialze_tensorboard.py
Expand Down
18 changes: 5 additions & 13 deletions tensorboard/plugins/audio/tf_audio_dashboard/tf-audio-loader.html
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@
import {getRouter} from "../tf-backend/router.js";
import {formatDate} from '../tf-card-heading/util.js';
import {runsColorScale} from "../tf-color-scale/colorScale.js";
import * as urlPathHelpers from "../tf-backend/urlPathHelpers.js";

Polymer({
is: "tf-audio-loader",
Expand Down Expand Up @@ -235,19 +234,12 @@
// The route lacks GET parameters. Just use ours.
query = '?' + query;
}
if (getRouter().isDemoMode()) {
query = urlPathHelpers.demoify(query);
}

let individualAudioURL = route + query;
if (getRouter().isDemoMode()) {
individualAudioURL += '.png';
} else {
// Include wall_time just to disambiguate the URL and force
// the browser to reload the audio when the URL changes. The
// backend doesn't care about the value.
individualAudioURL += `&ts=${audioMetadata.wall_time}`;
}
// Include wall_time just to disambiguate the URL and force
// the browser to reload the audio when the URL changes. The
// backend doesn't care about the value.
const individualAudioURL = `${route}${query}&ts=${audioMetadata.wall_time}`;

return {
wall_time: formatDate(new Date(audioMetadata.wall_time * 1000)),
step: audioMetadata.step,
Expand Down
18 changes: 5 additions & 13 deletions tensorboard/plugins/image/tf_image_dashboard/tf-image-loader.html
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@
import {getRouter} from "../tf-backend/router.js";
import {formatDate} from '../tf-card-heading/util.js';
import {runsColorScale} from "../tf-color-scale/colorScale.js";
import * as urlPathHelpers from "../tf-backend/urlPathHelpers.js";

Polymer({
is: "tf-image-loader",
Expand Down Expand Up @@ -330,19 +329,12 @@
// The route lacks GET parameters. Just use ours.
query = '?' + query;
}
if (getRouter().isDemoMode()) {
query = urlPathHelpers.demoify(query);
}

let individualImageUrl = route + query;
if (getRouter().isDemoMode()) {
individualImageUrl += '.png';
} else {
// Include wall_time just to disambiguate the URL and force
// the browser to reload the image when the URL changes. The
// backend doesn't care about the value.
individualImageUrl += `&ts=${imageMetadata.wall_time}`;
}
// Include wall_time just to disambiguate the URL and force
// the browser to reload the image when the URL changes. The
// backend doesn't care about the value.
const individualImageUrl = `${route}${query}&ts=${imageMetadata.wall_time}`;

return {
width: imageMetadata.width,
height: imageMetadata.height,
Expand Down